From 4db2cf71c04d66922c4fee3786d186e877520533 Mon Sep 17 00:00:00 2001 From: "sojung.kim" Date: Mon, 13 May 2024 00:47:06 +0900 Subject: [PATCH 1/6] fix(#2341): Initialize slots with empty BitSet in RedisClusterNode's constructors --- .../core/cluster/models/partitions/RedisClusterNode.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java b/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java index e1cef2fdce..0053ea6ea7 100644 --- a/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java +++ b/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java @@ -103,8 +103,7 @@ public RedisClusterNode(RedisURI uri, String nodeId, boolean connected, String s this.configEpoch = configEpoch; this.replOffset = -1; - this.slots = new BitSet(slots.length()); - this.slots.or(slots); + this.slots = slots != null ? slots : new BitSet(0); setFlags(flags); } @@ -123,8 +122,9 @@ public RedisClusterNode(RedisClusterNode redisClusterNode) { this.replOffset = redisClusterNode.replOffset; this.aliases.addAll(redisClusterNode.aliases); - if (redisClusterNode.slots != null && !redisClusterNode.slots.isEmpty()) { - this.slots = new BitSet(SlotHash.SLOT_COUNT); + this.slots = redisClusterNode.slots != null ? new BitSet(SlotHash.SLOT_COUNT) : new BitSet(0); + + if (redisClusterNode.slots != null) { this.slots.or(redisClusterNode.slots); } From f5fc05bf34977088d4b3f289073b5a4a99c915ad Mon Sep 17 00:00:00 2001 From: "sojung.kim" Date: Mon, 13 May 2024 00:49:01 +0900 Subject: [PATCH 2/6] test(#2341): Add test cases for slot initialization in RedisClusterNode constructors --- .../partitions/RedisClusterNodeUnitTests.java | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java index 4c0d02fc53..af876aa4d3 100644 --- a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java @@ -3,6 +3,8 @@ import static org.assertj.core.api.Assertions.*; import java.util.Arrays; +import java.util.BitSet; +import java.util.Collections; import org.junit.jupiter.api.Test; @@ -16,6 +18,55 @@ */ class RedisClusterNodeUnitTests { + @Test + void shouldCreateNodeWithEmptySlots() { + + BitSet slots = new BitSet(); + RedisClusterNode node = new RedisClusterNode(RedisURI.create("localhost", 6379), "1", true, null, 0, 0, 0, slots, + Collections.emptySet()); + assertThat(node.getSlots()).isEmpty(); + assertThat(node.getSlots()).isNotNull(); + } + + @Test + void shouldCreateNodeWithNonEmptySlots() { + + BitSet slots = new BitSet(); + slots.set(1); + slots.set(2); + RedisClusterNode node = new RedisClusterNode(RedisURI.create("localhost", 6379), "1", true, null, 0, 0, 0, slots, + Collections.emptySet()); + + assertThat(node.getSlots()).containsExactly(1, 2); + } + + @Test + void shouldCopyNodeWithEmptySlots() { + + BitSet slots = new BitSet(); + RedisClusterNode originalNode = new RedisClusterNode(RedisURI.create("localhost", 6379), "1", true, null, 0, 0, 0, + slots, Collections.emptySet()); + + RedisClusterNode copiedNode = new RedisClusterNode(originalNode); + + assertThat(copiedNode.getSlots()).isEmpty(); + assertThat(copiedNode.getSlots()).isNotNull(); + } + + @Test + void shouldCopyNodeWithNonEmptySlots() { + + BitSet slots = new BitSet(); + slots.set(1); + slots.set(2); + RedisClusterNode originalNode = new RedisClusterNode(RedisURI.create("localhost", 6379), "1", true, null, 0, 0, 0, + slots, Collections.emptySet()); + + RedisClusterNode copiedNode = new RedisClusterNode(originalNode); + + assertThat(copiedNode.getSlots()).containsExactly(1, 2); + } + @Test void shouldCopyNode() { From 4ab70ad9f4aa212d32af07cf0cac29dc29986023 Mon Sep 17 00:00:00 2001 From: "sojung.kim" Date: Sat, 1 Jun 2024 15:49:28 +0900 Subject: [PATCH 3/6] fix(redis#2341): Initialize RedisClusterNode slots with SlotHash.SLOT_COUNT --- .../core/cluster/models/partitions/RedisClusterNode.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java b/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java index 0053ea6ea7..7645281c24 100644 --- a/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java +++ b/src/main/java/io/lettuce/core/cluster/models/partitions/RedisClusterNode.java @@ -103,7 +103,11 @@ public RedisClusterNode(RedisURI uri, String nodeId, boolean connected, String s this.configEpoch = configEpoch; this.replOffset = -1; - this.slots = slots != null ? slots : new BitSet(0); + this.slots = new BitSet(SlotHash.SLOT_COUNT); + + if (slots != null) { + this.slots.or(slots); + } setFlags(flags); } @@ -122,7 +126,7 @@ public RedisClusterNode(RedisClusterNode redisClusterNode) { this.replOffset = redisClusterNode.replOffset; this.aliases.addAll(redisClusterNode.aliases); - this.slots = redisClusterNode.slots != null ? new BitSet(SlotHash.SLOT_COUNT) : new BitSet(0); + this.slots = new BitSet(SlotHash.SLOT_COUNT); if (redisClusterNode.slots != null) { this.slots.or(redisClusterNode.slots); From e5ea2953bc6f79cc688eda5f36891edf0b6cf4f3 Mon Sep 17 00:00:00 2001 From: "sojung.kim" Date: Sat, 1 Jun 2024 15:57:24 +0900 Subject: [PATCH 4/6] chore(redis#2341): Adjust the formatting --- .../cluster/models/partitions/RedisClusterNodeUnitTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java index af876aa4d3..090e3dbad0 100644 --- a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java @@ -24,6 +24,7 @@ void shouldCreateNodeWithEmptySlots() { BitSet slots = new BitSet(); RedisClusterNode node = new RedisClusterNode(RedisURI.create("localhost", 6379), "1", true, null, 0, 0, 0, slots, Collections.emptySet()); + assertThat(node.getSlots()).isEmpty(); assertThat(node.getSlots()).isNotNull(); } From c5b8720921835c1d7097c3aa3e0596d7531fdde5 Mon Sep 17 00:00:00 2001 From: "sojung.kim" Date: Sat, 1 Jun 2024 16:08:00 +0900 Subject: [PATCH 5/6] test(redis#2341):Add test cases for hasSameSlotsAs() --- .../partitions/RedisClusterNodeUnitTests.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java index 090e3dbad0..ec21559197 100644 --- a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java @@ -5,6 +5,7 @@ import java.util.Arrays; import java.util.BitSet; import java.util.Collections; +import java.util.HashSet; import org.junit.jupiter.api.Test; @@ -68,6 +69,42 @@ void shouldCopyNodeWithNonEmptySlots() { assertThat(copiedNode.getSlots()).containsExactly(1, 2); } + @Test + public void testHasSameSlotsAs() { + + BitSet slots1 = new BitSet(SlotHash.SLOT_COUNT); + slots1.set(1); + slots1.set(2); + + BitSet slots2 = new BitSet(SlotHash.SLOT_COUNT); + slots2.set(1); + slots2.set(2); + + RedisClusterNode node1 = new RedisClusterNode(RedisURI.create("localhost", 6379), "nodeId1", true, "slaveOf", 0L, 0L, + 0L, slots1, new HashSet<>()); + RedisClusterNode node2 = new RedisClusterNode(RedisURI.create("localhost", 6379), "nodeId2", true, "slaveOf", 0L, 0L, + 0L, slots2, new HashSet<>()); + + assertThat(node1.hasSameSlotsAs(node2)).isTrue(); + } + + @Test + public void testHasDifferentSlotsAs() { + + BitSet slots1 = new BitSet(SlotHash.SLOT_COUNT); + slots1.set(1); + + BitSet slots2 = new BitSet(SlotHash.SLOT_COUNT); + slots2.set(2); + + RedisClusterNode node1 = new RedisClusterNode(RedisURI.create("localhost", 6379), "nodeId1", true, "slaveOf", 0L, 0L, + 0L, slots1, new HashSet<>()); + RedisClusterNode node2 = new RedisClusterNode(RedisURI.create("localhost", 6379), "nodeId2", true, "slaveOf", 0L, 0L, + 0L, slots2, new HashSet<>()); + + assertThat(node1.hasSameSlotsAs(node2)).isFalse(); + } + @Test void shouldCopyNode() { From 18572c06a3f13e575606242787a2b6bf2877cc60 Mon Sep 17 00:00:00 2001 From: "sojung.kim" Date: Fri, 14 Jun 2024 19:48:55 +0900 Subject: [PATCH 6/6] fix(#2341): Clone node2 from node1 using the RedisClusterNode constructor and compare the two clusters with hasSameSlotsAs() --- .../partitions/RedisClusterNodeUnitTests.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java index ec21559197..8c9ed71688 100644 --- a/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/models/partitions/RedisClusterNodeUnitTests.java @@ -72,18 +72,14 @@ void shouldCopyNodeWithNonEmptySlots() { @Test public void testHasSameSlotsAs() { - BitSet slots1 = new BitSet(SlotHash.SLOT_COUNT); - slots1.set(1); - slots1.set(2); - - BitSet slots2 = new BitSet(SlotHash.SLOT_COUNT); - slots2.set(1); - slots2.set(2); + BitSet emptySlots = new BitSet(SlotHash.SLOT_COUNT); + emptySlots.set(1); + emptySlots.set(2); RedisClusterNode node1 = new RedisClusterNode(RedisURI.create("localhost", 6379), "nodeId1", true, "slaveOf", 0L, 0L, - 0L, slots1, new HashSet<>()); - RedisClusterNode node2 = new RedisClusterNode(RedisURI.create("localhost", 6379), "nodeId2", true, "slaveOf", 0L, 0L, - 0L, slots2, new HashSet<>()); + 0L, emptySlots, new HashSet<>()); + + RedisClusterNode node2 = new RedisClusterNode(node1); assertThat(node1.hasSameSlotsAs(node2)).isTrue(); }