From 9d47c48e227ab5cdd9709fe49aaa97db3815ddf2 Mon Sep 17 00:00:00 2001 From: ChenGuanqun Date: Tue, 20 Nov 2018 21:55:21 +0800 Subject: [PATCH] DATAREDIS-890 - Randomize Redis Cluster node order before topology retrieval. We now shuffle the set of Redis Cluster nodes before retrieving the topology. This change reduces load on the first node in the known nodes set. Original pull request: #373. --- .../jedis/JedisClusterConnection.java | 7 ++++++- .../jedis/JedisClusterConnectionUnitTests.java | 17 +++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java index 81e53258a..91a6d774e 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java @@ -24,7 +24,9 @@ import redis.clients.jedis.JedisCluster; import redis.clients.jedis.JedisClusterConnectionHandler; import redis.clients.jedis.JedisPool; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -64,6 +66,7 @@ import org.springframework.util.Assert; * @author Christoph Strobl * @author Mark Paluch * @author Ninad Divadkar + * @author Tao Chen * @since 1.7 */ public class JedisClusterConnection implements DefaultedRedisClusterConnection { @@ -982,7 +985,9 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection { Map errors = new LinkedHashMap<>(); - for (Entry entry : cluster.getClusterNodes().entrySet()) { + List> list = new ArrayList<>(cluster.getClusterNodes().entrySet()); + Collections.shuffle(list); + for (Entry entry : list) { Jedis jedis = null; diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java index b3c6404d4..49d3524d3 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java @@ -103,6 +103,8 @@ public class JedisClusterConnectionUnitTests { when(node3PoolMock.getResource()).thenReturn(con3Mock); when(con1Mock.clusterNodes()).thenReturn(CLUSTER_NODES_RESPONSE); + when(con2Mock.clusterNodes()).thenReturn(CLUSTER_NODES_RESPONSE); + when(con3Mock.clusterNodes()).thenReturn(CLUSTER_NODES_RESPONSE); clusterMock.setConnectionHandler(connectionHandlerMock); connection = new JedisClusterConnection(clusterMock); @@ -140,7 +142,6 @@ public class JedisClusterConnectionUnitTests { connection.clusterForget(CLUSTER_NODE_2); verify(con1Mock, times(1)).clusterForget(CLUSTER_NODE_2.getId()); - verifyZeroInteractions(con2Mock); verify(con3Mock, times(1)).clusterForget(CLUSTER_NODE_2.getId()); } @@ -150,8 +151,8 @@ public class JedisClusterConnectionUnitTests { connection.clusterReplicate(CLUSTER_NODE_1, CLUSTER_NODE_2); verify(con2Mock, times(1)).clusterReplicate(CLUSTER_NODE_1.getId()); - verify(con1Mock, times(1)).clusterNodes(); - verify(con1Mock, times(1)).close(); + verify(con1Mock, atMost(1)).clusterNodes(); + verify(con1Mock, atMost(1)).close(); verifyZeroInteractions(con1Mock); } @@ -312,10 +313,10 @@ public class JedisClusterConnectionUnitTests { connection.time(CLUSTER_NODE_2); verify(con2Mock, times(1)).time(); - verify(con2Mock, times(1)).close(); - verify(con1Mock, times(1)).clusterNodes(); - verify(con1Mock, times(1)).close(); - verifyZeroInteractions(con1Mock, con3Mock); + verify(con2Mock, atLeast(1)).close(); + verify(con1Mock, atMost(1)).clusterNodes(); + verify(con1Mock, atMost(1)).close(); + } @Test // DATAREDIS-315 @@ -334,7 +335,7 @@ public class JedisClusterConnectionUnitTests { connection.resetConfigStats(CLUSTER_NODE_2); verify(con2Mock, times(1)).configResetStat(); - verify(con2Mock, times(1)).close(); + verify(con2Mock, atLeast(1)).close(); verify(con1Mock, never()).configResetStat(); verify(con3Mock, never()).configResetStat(); }