When using SimpleRedisDocumentRepository.saveAll or SimpleRedisEnhancedRepository.saveAll, any errors that occur on write requests in the pipeline are swallowed.
We are seeing, when calling SimpleRedisDocumentRepository.saveAll, that the database reaches its maximum memory and not all documents are saved to the database. The redis logs print these warnings:
3344574:S 10 Oct 2024 09:59:40.407 # WARNING: the new maxmemory value set via CONFIG SET (1208585328) is smaller than the current memory usage (2068044275). This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy.
which indicates that the database must be returning OOM errors to write requests. Yet no exception is thrown from saveAll.
We observed the issue with SimpleRedisDocumentRepository, but the code for SimpleRedisEnhancedRepository indicates it will have the same issue.
In both classes, the saveAll method uses a pipeline, but does not check for errors or return the pipeline responses to allow the caller to check for errors.
SimpleRedisDocumentRepository.saveAll calls pipeline.sendCommand which returns a Response. And SimpleRedisEnhancedRepository.saveAll calls pipeline.hmset which also returns a Response. After the pipeline has closed, you can call Response.get to get the response data, and if there was an error then Response.get throws an exception. We could collect up these Responses and iterate over them calling get on each after syncing the pipeline.
Or, instead of using pipeline.sync we could use syncAndReturnAll, which returns a list of all response objects. If any request resulted in an error, the corresponding response object in the list will be a JedisDataException. We could iterate the list and throw any exception found.
Alternatively, saveAll could return a list of Responses or return the list returned by syncAndReturnAll, so the caller can check for errors. But this would mean changing the method signature.
When using
SimpleRedisDocumentRepository.saveAllorSimpleRedisEnhancedRepository.saveAll, any errors that occur on write requests in the pipeline are swallowed.We are seeing, when calling
SimpleRedisDocumentRepository.saveAll, that the database reaches its maximum memory and not all documents are saved to the database. The redis logs print these warnings:3344574:S 10 Oct 2024 09:59:40.407 # WARNING: the new maxmemory value set via CONFIG SET (1208585328) is smaller than the current memory usage (2068044275). This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy.which indicates that the database must be returning OOM errors to write requests. Yet no exception is thrown from saveAll.We observed the issue with
SimpleRedisDocumentRepository, but the code forSimpleRedisEnhancedRepositoryindicates it will have the same issue.In both classes, the
saveAllmethod uses a pipeline, but does not check for errors or return the pipeline responses to allow the caller to check for errors.SimpleRedisDocumentRepository.saveAllcallspipeline.sendCommandwhich returns aResponse. AndSimpleRedisEnhancedRepository.saveAllcallspipeline.hmsetwhich also returns aResponse. After the pipeline has closed, you can callResponse.getto get the response data, and if there was an error thenResponse.getthrows an exception. We could collect up theseResponses and iterate over them callinggeton each after syncing the pipeline.Or, instead of using
pipeline.syncwe could usesyncAndReturnAll, which returns a list of all response objects. If any request resulted in an error, the corresponding response object in the list will be aJedisDataException. We could iterate the list and throw any exception found.Alternatively,
saveAllcould return a list ofResponses or return the list returned bysyncAndReturnAll, so the caller can check for errors. But this would mean changing the method signature.