Cluster state is not reloaded on connection refused

[bjorngylling]

Expected Behavior

Given a master node in the cluster is shut down and the slave becomes the master, when the cluster client tries to get a key on the old master node the client should react to the network error and retry the get command against the new master node.

Current Behavior

Given a master node in the cluster is shut down and the slave becomes the master, when the cluster client tries to get a key on the old master node it continues its connection attempts against the old master until ClusterStateReloadInterval has passed. This becomes even more problematic since v9.22 where the default value was adjusted from 10s to 60s.

Possible Solution

Reload the cluster state immediately or within a very short duration on network errors perhaps? I'm not sure but the current behaviour seems like a bug given that the cluster is already aware and has changed the master node, but the client refuses to adjust for quite some time.

Steps to Reproduce

1. A running redis cluster
2. A go-redis clusterclient with the default configuration
3. Set the value for a key
4. Issue a CLUSTER FAILOVER TAKEOVER for the slave with that key slot so it becomes the master. This is done to avoid the cluster-node-timeout delay, we could of course skip this step to better simulate an uncontrolled node failure but that adds another timing aspect into the mix (cluster-node-timeout)
5. Shut down the old master node for that key slot
6. Try to get the key, the go-redis client will get connection refused, retry 3 times and then give up

By increasing MaxRedirects to a high value (I used 100) we can make the client retry the connection until it succeeds. If I time the get command in step 6 above with MaxRedirects=100 it succeeds after 62 seconds. If I rerun the scenario but I set ClusterStateReloadInterval to 10 seconds the get will succeed after 11 seconds.

Context (Environment)

This behaviour makes the client react unreasonably slow to changes in the cluster and becomes even worse with the default configuration change in v9.22. Perhaps we can set ClusterStateReloadInterval to a very low value, but I suspect that causes many unnecessary state reloads during normal operations?

[official-burak]

#3990 takes the same path as MOVED: LazyReload on connection refused, rather than a synchronous CLUSTER SLOTS in the retry loop.

A sync reload would make the next Get in that same command see the new map, but it also blocks the caller and is unguarded, so concurrent failovers can stampede. Waiting inside Get for an in-flight reload is a possible follow-up.

Until the async refresh lands, a command can still see the previous slot map for that retry budget. After it lands, later commands use the new master, instead of waiting for ClusterStateReloadInterval (default 60s).