From 1d79ff521765d6bb6d8d6d2b941a093020c80c5f 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 5a09e9e0b..845975809 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 @@ -23,7 +23,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; @@ -60,6 +62,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 { @@ -884,7 +887,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(); }