1 files changed, 36 insertions, 33 deletions
diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
index 760c326cf5d..1211624e3d6 100644
--- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
+++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
@@ -17,6 +17,7 @@ import java.util.Arrays;
 import java.util.concurrent.Phaser;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 
 /**
@@ -50,26 +51,31 @@ public class ReconfigurerTest {
         ZookeeperServerConfig nextConfig = createConfig(5, true);
         reconfigurer.startOrReconfigure(nextConfig);
         assertEquals("node1:2181", reconfigurer.connectionSpec());
-        assertEquals("server.0=node0:2182:2183;2181,server.1=node1:2182:2183;2181,server.2=node2:2182:2183;2181,server.3=node3:2182:2183;2181,server.4=node4:2182:2183;2181",
-                     reconfigurer.servers());
-        assertEquals(2, reconfigurer.reconfigurations());
+        assertEquals("3=node3:2182:2183;2181,4=node4:2182:2183;2181", reconfigurer.joiningServers());
+        assertNull("No servers are leaving", reconfigurer.leavingServers());
+        assertEquals(1, reconfigurer.reconfigurations());
+        assertSame(nextConfig, reconfigurer.activeConfig());
+
+        // No reconfiguration happens with same config
+        reconfigurer.startOrReconfigure(nextConfig);
+        assertEquals(1, reconfigurer.reconfigurations());
         assertSame(nextConfig, reconfigurer.activeConfig());
 
         // Cluster shrinks
         nextConfig = createConfig(3, true);
         reconfigurer.startOrReconfigure(nextConfig);
-        assertEquals(3, reconfigurer.reconfigurations());
+        assertEquals(2, reconfigurer.reconfigurations());
         assertEquals("node1:2181", reconfigurer.connectionSpec());
-        assertEquals("server.0=node0:2182:2183;2181,server.1=node1:2182:2183;2181,server.2=node2:2182:2183;2181",
-                     reconfigurer.servers());
+        assertNull("No servers are joining", reconfigurer.joiningServers());
+        assertEquals("3,4", reconfigurer.leavingServers());
         assertSame(nextConfig, reconfigurer.activeConfig());
 
         // Cluster loses node1, but node3 joins. Indices are shuffled.
         nextConfig = createConfig(3, true, 1);
         reconfigurer.startOrReconfigure(nextConfig);
-        assertEquals(4, reconfigurer.reconfigurations());
-        assertEquals("server.0=node0:2182:2183;2181,server.1=node2:2182:2183;2181,server.2=node3:2182:2183;2181",
-                     reconfigurer.servers());
+        assertEquals(3, reconfigurer.reconfigurations());
+        assertEquals("1=node2:2182:2183;2181,2=node3:2182:2183;2181", reconfigurer.joiningServers());
+        assertEquals("1,2", reconfigurer.leavingServers());
         assertSame(nextConfig, reconfigurer.activeConfig());
     }
 
@@ -83,9 +89,9 @@ public class ReconfigurerTest {
         ZookeeperServerConfig nextConfig = createConfig(5, true);
         reconfigurer.startOrReconfigure(nextConfig);
         assertEquals("node1:2181", reconfigurer.connectionSpec());
-        assertEquals("server.0=node0:2182:2183;2181,server.1=node1:2182:2183;2181,server.2=node2:2182:2183;2181,server.3=node3:2182:2183;2181,server.4=node4:2182:2183;2181",
-                     reconfigurer.servers());
-        assertEquals(2, reconfigurer.reconfigurations());
+        assertEquals("3=node3:2182:2183;2181,4=node4:2182:2183;2181", reconfigurer.joiningServers());
+        assertNull("No servers are leaving", reconfigurer.leavingServers());
+        assertEquals(1, reconfigurer.reconfigurations());
         assertSame(nextConfig, reconfigurer.activeConfig());
     }
 
@@ -106,27 +112,24 @@ public class ReconfigurerTest {
        reconfigurer.shutdown();
     }
 
-    private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration, int... retiredIndices) {
-        Arrays.sort(retiredIndices);
+    private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration, int... skipIndices) {
+        Arrays.sort(skipIndices);
         ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
         builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
         builder.myidFile(idFile.getAbsolutePath());
         for (int i = 0, index = 0; i < numberOfServers; i++, index++) {
-            boolean retired = Arrays.binarySearch(retiredIndices, index) >= 0;
-            if (retired) i--;
-            builder.server(newServer(i, "node" + index, retired));
+            while (Arrays.binarySearch(skipIndices, index) >= 0) index++;
+            builder.server(newServer(i, "node" + index));
         }
-
         builder.myid(0);
         builder.dynamicReconfiguration(dynamicReconfiguration);
         return builder.build();
     }
 
-    private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName, boolean retired) {
+    private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName) {
         ZookeeperServerConfig.Server.Builder builder = new ZookeeperServerConfig.Server.Builder();
         builder.id(id);
         builder.hostname(hostName);
-        builder.retired(retired);
         return builder;
     }
 
@@ -139,7 +142,6 @@ public class ReconfigurerTest {
     private static class TestableReconfigurer extends Reconfigurer implements VespaZooKeeperServer {
 
         private final TestableVespaZooKeeperAdmin zooKeeperAdmin;
-        private final Phaser phaser = new Phaser(2);
         private QuorumPeer serverPeer;
 
         TestableReconfigurer(TestableVespaZooKeeperAdmin zooKeeperAdmin) {
@@ -154,16 +156,19 @@ public class ReconfigurerTest {
         }
 
         void startOrReconfigure(ZookeeperServerConfig newConfig) {
-            serverPeer = startOrReconfigure(newConfig, this, MockQuorumPeer::new);
-            phaser.arriveAndDeregister();
+            startOrReconfigure(newConfig, this, MockQuorumPeer::new, peer -> serverPeer = peer);
         }
 
         String connectionSpec() {
             return zooKeeperAdmin.connectionSpec;
         }
 
-        String servers() {
-            return zooKeeperAdmin.servers;
+        String joiningServers() {
+            return zooKeeperAdmin.joiningServers;
+        }
+
+        String leavingServers() {
+            return zooKeeperAdmin.leavingServers;
         }
 
         int reconfigurations() {
@@ -172,14 +177,10 @@ public class ReconfigurerTest {
 
         @Override
         public void shutdown() {
-            phaser.arriveAndAwaitAdvance();
             serverPeer.shutdown(Duration.ofSeconds(1)); }
 
         @Override
-        public void start(Path configFilePath) {
-            phaser.arriveAndAwaitAdvance();
-            serverPeer.start(configFilePath);
-        }
+        public void start(Path configFilePath) { serverPeer.start(configFilePath); }
 
         @Override
         public boolean reconfigurable() {
@@ -191,7 +192,8 @@ public class ReconfigurerTest {
     private static class TestableVespaZooKeeperAdmin implements VespaZooKeeperAdmin {
 
         String connectionSpec;
-        String servers;
+        String joiningServers;
+        String leavingServers;
         int reconfigurations = 0;
 
         private int failures = 0;
@@ -203,11 +205,12 @@ public class ReconfigurerTest {
         }
 
         @Override
-        public void reconfigure(String connectionSpec, String servers) throws ReconfigException {
+        public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException {
             if (++attempts < failures)
                 throw new ReconfigException("Reconfig failed");
             this.connectionSpec = connectionSpec;
-            this.servers = servers;
+            this.joiningServers = joiningServers;
+            this.leavingServers = leavingServers;
             this.reconfigurations++;
         }