mp911de
commented
1 year ago Let's take a step back before jumping right into the code.
We default replOffset to -1 to indicate the absence of a replication offset. Lettuce can fetch topology details either from the initial seed nodes used to construct the cluster client or contact all cluster nodes. In the latter case, we can obtain all repl offsets, in the first case we only have repl offsets for the seed nodes.
If we'd default to zero as the initial value, we would stop considering replicas as read candidates.
We considered switching to CLUSTER SHARDS but that command stopped reporting the MYSELF node with Redis 7.2 so that isn't an option really.
I currently do not see a good way out to filter nodes in LOADING state without querying all Redis Cluster nodes. Let me know if I missed something.
barshaul
mannitrkl2006
srgsanky
Bug Report
Current Behavior
Lettuce is filtering out nodes in LOADING state by checking their master_repl_offset filed == 0. However, in the scenarios described below nodes' replOffset can be left with -1L value. Thus, a loading replica with replOffset == -1, would be considered as a valid read candidate and Lettuce will continue to query it, resulting in LOADING error raised back to the user.
When the replOffset is not being initialized properly?
Lets examine the getNodeSpecificViews function in DefaultClusterTopologyRefresh.java:
In the first for loop, if a specific node, lets call it X, is not a part of
nodes, but is found in other nodes' nodeTopologyView.getPartitions() (e.g., when a new node is added to the cluster topology), then X will be found inallNodes, but not inself. So, in the second loop we will skip this node and won't set it's replOffset:Another scenario is when node Y is a part of
node, but this node is currently loading. When a node is loading, CLUSTER NODES command will return a LOADING error. Therefore, we will skip adding this node toselfin the first loop because nodeTopologyView.isAvailable() will be false. However, the loading node will be present in other nodes CLUSTER NODES output, and therefore will be added toallNodes. Again, we'll skip this node in the second loop and won't set it's replOffset:In both cases, the created topology will contain nodes with replOffset == -1 at the end of the creation of the client's topology.
Stack trace
I added logs to Lettuce to show the behavior, notice the 'Input Code
Input Code
```java // your code here; ```Expected behavior/code
A loading node should not be considered as a read candidate.
Environment
Possible Solution
There are couple of options:
Mitigation for users who currently facing this issue:
1. Catch LOADING errors and route them back to the primary [preferred]
Downsides:
In this mitigation we keep the loading replica in the client’s topology view, so the client will continue to refer to this replica as a read candidate and route read requests to it. This means that all commands that will fail due to LOADING error will have a doubled latency - once the time it took to send it to the replica, and second the time to send it to the primary. However, it will enforce not getting a full downtime for read queries. Another downside of this method is that all traffic that was routed to the replica will now be routed to the primary, which can increase the load on the primary node. If there’re other replicas, we would have wanted to be able to route the failed command to another replica, or at least to randomly choose a node from the primary and other replicas. However, AFAIK, Lettuce currently doesn’t provider information about which of the nodes returned the LOADING error, so we don’t have a way to select all nodes in this shard and exclude only the loading ones. I’m still waiting for response about if it’s possible on the git’s issue.
2. Filter out replicas with replOffset == -1, while enabling the periodic topology updates:
Downsides:
This mitigation means that we’ll filter out new replicas, regardless to their loading state, and replaced replicas during their loading state. The new replicas will be added back to the cluster topology only in the next periodic topology update. Consequently, these replicas will remain inaccessible for read queries for an extended period equal to or less than the duration between periodic topology checks. Moreover, if an existing replica gets into a LOADING state, the client will continue to route requests to it until it will remove it in the next topology check. If the cluster is large, setting the duration for the topology updates too low may impact performance, as the length of the CLUSTER NODES output is affected by the cluster size.