RedisClusterNode without slots is never considered having same slots as an equal object

[y2klyf]

Bug Report

Current Behavior

When using lettuce 5.1.7 connect to a redis cluster of 6 nodes (3 master and 3 slaves), we noticed the following log message all the time right after each periodic refresh:

18:41:16.476 [lettuce-eventExecutorLoop-1-1] DEBUG io.lettuce.core.cluster.RedisClusterClient - Using a new cluster topology

upon further debugging, we noticed that
TopologyComparators.isChanged return TRUE all the time.

There is a new change in 5.1.7 that suppose to improve performance when comparing node slots.
408f771#diff-d4283d9d34b726c4de2d961ddde6186fR259

However, this.slot or other.slot is always NULL for slave nodes and hence it always think topology has changed.

The logic will not work for cluster that has slave (replicate) node, as any slave node will not have slot field being populated.

A sample cluster nodes command also indicates that slave nodes does not include slot info

cluster nodes
45f9a7d618586e09756205e47a6df9a6d674a2c8 127.0.0.1:7002@17002 master - 0 1564104024073 2 connected 5461-10922
aaff46e9fa198fd9ba9281ea493d37cdc5f0b17c 127.0.0.1:7003@17003 master - 0 1564104024574 3 connected 10923-16383 
a5e31b1c6832adf658d08a3fb44de2512177023c 127.0.0.1:7004@17004 slave 45f9a7d618586e09756205e47a6df9a6d674a2c8 0 1564104024073 4 connected
b3f0f7df704f33fbdfc344649e99e1fa1a992de0 127.0.0.1:7001@17001 myself,master - 0 1564104023000 1 connected 0-5460
de13aee6d5721345544d13dc816af17c23fe2374 127.0.0.1:7006@17006 slave b3f0f7df704f33fbdfc344649e99e1fa1a992de0 0 1564104024073 6 connected
5bf282cbd434bb80435c466d93f92b98bd90cd3f 127.0.0.1:7005@17005 slave aaff46e9fa198fd9ba9281ea493d37cdc5f0b17c 0 1564104024574 5 connected

Expected behavior/code

Periodic topology refresh should not reload partitions if topology has not changed.
Currently the system maybe busy keep refreshing partition information affect actual command execution.

Environment

• Lettuce version(s): [5.1.7]
• Redis version: [5.0.4]

Possible Solution

check for slave flag, if so, slot == null should not be treated as slot not the same.
or
use getSlot() which will replace null with empty set.

Additional context

[y2klyf]

Additionally, ClusterTopologyRefreshSchedulear call RedisClusterClient.reloadPartitions()
https://github.com/lettuce-io/lettuce-core/blob/master/src/main/java/io/lettuce/core/cluster/RedisClusterClient.java#L783

The code however, will do
this.partitions.reload(loadedPartitions.getPartitions());
and
updatePartitionsInConnections();
regardless if topology is changed or not.

Maybe i misunderstand the concept of periodic topology refresh, it seem it always update the internal data structure even if there is no change?

[mp911de]

Thanks for report. This is indeed a bug. Unchanged topology should not trigger a topology changed event and we should early return instead of invalidating caches.

[mp911de]

That's fixed now.