jdeppe-pivotal commented on a change in pull request #6521: URL: https://github.com/apache/geode/pull/6521#discussion_r639972279
########## File path: geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/cluster/ClusterSlotsAndNodesDUnitTest.java ########## @@ -244,6 +249,42 @@ public void slotsAreNotMissingOrDuplicatedWhenPrimariesAreMoving() throws Except assertThat(iterations).isGreaterThan(0); } + @Test + public void hostAndPortInfoIsUnique_whenPrimariesAreMoving() throws Exception { + AtomicBoolean done = new AtomicBoolean(); + CompletableFuture<Void> startupShutdownFuture = executor.runAsync(() -> { + while (!done.get()) { + server2.stop(); + server2 = cluster.startRedisVM(2, locator.getPort()); + rebalanceAllRegions(server1); + } + }); + + long endTime = System.currentTimeMillis() + 60_000; + CompletableFuture<Integer> getSlotsFuture = executor.supplyAsync(() -> { + int iterations = 0; + + while (System.currentTimeMillis() < endTime) { + List<ClusterNode> nodes = ClusterNodes.parseClusterNodes(jedis1.clusterNodes()).getNodes(); + + if (nodes.size() != 2) { + continue; + } + + assertThat(nodes.get(0).port).isNotEqualTo(nodes.get(1).port); + iterations++; + } + + return iterations; Review comment: In practice it would probably be just fine, however in theory it might mess with cleanup. We still want the sequence below to remain the same, however if `getSlotsFuture.get()` ends up throwing an exception (because the assertion on iterations failed) it would also prevent the subsequent calls to `done.set(true)...`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org