RedisClusterClient is not retrying after connection failures

[upthewaterspout]

This is somewhat similar to #1789, however it is not related to a handshake failure.

I have a test that is using a RedisClusterClient that is doing set operations to redis in cluster mode. At the same time the test is killing and restarting redis servers. What I see is that sometimes the I get this failure from lettuce (I think due to a server crashing):

java.util.concurrent.ExecutionException: io.netty.channel.unix.Errors$NativeIoException: readAddress(..) failed: Connection reset by peer

I was expecting lettuce to retry the operation until it succeeds, discovering the new location of the slots and sending the command there if necessary.

I created a reproducible test case in the lettuce test suite which is available here. This particular test is not actually killing servers but it is simulating a failure by calling CLIENT KILL on the server to drop the client connection. Here's an example failure from the test

java.util.concurrent.ExecutionException: io.netty.channel.unix.Errors$NativeIoException: readAddress(..) failed: Connection reset by peer

	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:395)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2022)
	at io.lettuce.core.cluster.topology.ConnectionFailureRetryIntegrationTests.retriesOperationWhenConnectionIsKilled(ConnectionFailureRetryIntegrationTests.java:125)
Caused by: io.netty.channel.unix.Errors$NativeIoException: readAddress(..) failed: Connection reset by peer

Should I turn this into a bug report, or am I doing or expecting something wrong from lettuce?

[mp911de]

Exceptions that happen when sending a command or reading the response bubble as exceptions to indicate that a failure has happened and that requires special attention. On the sending side, it could be that the command has been sent but we don't know. Errors on the reading side typically indicate that a command has been processed on the server but for some reason the client wasn't able to fully consume the response.

Silently retrying such commands is prone to cause duplicate actions on the server.

[upthewaterspout]

Would it be reasonable to say that lettuce is only providing at-most-once execution then?

The documentation seems to imply that lettuce provides at-least-once execution. From the section on Failures and at-least-once execution:

A retry mechanism re-executes commands that are not successfully completed if a network failure occurs

and

at-least-once execution ensures a higher consistency level than at-most-once but comes with some caveats:
    Commands can be executed more than once
    Higher usage of resources since commands are buffered and sent again after reconnect

Perhaps this documentation is out of date or inaccurate? For us it would be preferable to have the "at-least-once" behavior described in the documentation, but I do understand the concerns about executing commands more than once.

[mp911de]

at-most-once and at-least-once apply in the context of not knowing whether a command was performed or not and whether a command should be retried. This case is a specialization as we receive feedback from the network in the sense of Connection reset by peer. While we still do not know what might have happened, we at least can propagate a cause and that is used to complete a command.

[upthewaterspout]

at-most-once and at-least-once apply in the context of not knowing whether a command was performed or not and whether a command should be retried

Does this mean there are cases where lettuce will retry and potentially cause duplicate events on the server, but just not this particular case? I feel like if I already have to handle the possibility of duplicate events I'd rather lettuce retry in all cases.

If lettuce is intentionally throwing exceptions on network failures, do you have suggestions on how to write an application that avoids displaying errors to the end user when one redis server in my redis cluster crashes? Should I wrap all redis operations in some application level retry logic? What exceptions from lettuce should trigger a retry vs. be considered fatal?

[upthewaterspout]

BTW -Even though I'm questioning some of the behavior I really appreciate the level of detail in the lettuce documentation. It's really helpful that the docs address this situation. I was trying to look up what jedis does and there is just nothing. Thanks for all your effort!

[mp911de]

The docs are open source and maintained in our wiki. Feel free to edit it at https://github.com/lettuce-io/lettuce-core/wiki/Command-execution-reliability