Revert "Periodically reconfigure config server ZooKeeper cluster"

3 files changed, 4 insertions, 81 deletions

diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
index 1d47d890421..9c2a543d2ef 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
@@ -20,7 +20,7 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
- * Starts or reconfigures a ZooKeeper server as necessary. This is created as a component
+ * Starts zookeeper server and supports reconfiguring zookeeper cluster. Created as a component
  * without any config injected, to make sure that it is not recreated when config changes.
  *
  * @author hmusum
@@ -45,13 +45,12 @@ public class Reconfigurer extends AbstractComponent {
         this(vespaZooKeeperAdmin, new Sleeper());
     }
 
-    public Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin, Sleeper sleeper) {
+    Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin, Sleeper sleeper) {
         this.vespaZooKeeperAdmin = Objects.requireNonNull(vespaZooKeeperAdmin);
         this.sleeper = Objects.requireNonNull(sleeper);
         log.log(Level.FINE, "Created ZooKeeperReconfigurer");
     }
 
-    /** Start a ZooKeeper server or reconfigure a currently running cluster */
     void startOrReconfigure(ZookeeperServerConfig newConfig, VespaZooKeeperServer server,
                             Supplier<QuorumPeer> quorumPeerGetter, Consumer<QuorumPeer> quorumPeerSetter) {
         if (zooKeeperRunner == null) {
@@ -59,22 +58,10 @@ public class Reconfigurer extends AbstractComponent {
             zooKeeperRunner = startServer(newConfig, server);
         }
         quorumPeerSetter.accept(peer);
-        reconfigure(newConfig);
-    }
 
-    /** Reconfigure a running ZooKeeper cluster with given servers. This is a no-op if servers are unchanged */
-    public void reconfigure(List<ZooKeeperServer> wantedServers) {
-        ZookeeperServerConfig.Builder b = new ZookeeperServerConfig.Builder();
-        b.myid(-1) // Required by ZookeeperServerConfig, but not used for reconfiguration
-         .dynamicReconfiguration(true);
-        for (var server : wantedServers) {
-            ZookeeperServerConfig.Server.Builder serverBuilder = new ZookeeperServerConfig.Server.Builder();
-            serverBuilder.id(server.id())
-                         .hostname(server.hostname());
-            b.server(serverBuilder);
+        if (shouldReconfigure(newConfig)) {
+            reconfigure(newConfig);
         }
-        ZookeeperServerConfig newConfig = b.build();
-        reconfigure(newConfig);
     }
 
     ZookeeperServerConfig activeConfig() {
@@ -90,7 +77,6 @@ public class Reconfigurer extends AbstractComponent {
     private boolean shouldReconfigure(ZookeeperServerConfig newConfig) {
         if (!newConfig.dynamicReconfiguration()) return false;
         if (activeConfig == null) return false;
-        if (newConfig.server().isEmpty()) return false;
         return !newConfig.equals(activeConfig());
     }
 
@@ -100,9 +86,7 @@ public class Reconfigurer extends AbstractComponent {
         return runner;
     }
 
-    /** Reconfigure ZooKeeper cluster with given config, if necessary */
     private void reconfigure(ZookeeperServerConfig newConfig) {
-        if (!shouldReconfigure(newConfig)) return;
         Instant reconfigTriggered = Instant.now();
         // No point in trying to reconfigure if there is only one server in the new ensemble,
         // the others will be shutdown or are about to be shutdown
diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
deleted file mode 100644
index ef75d0b5da7..00000000000
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.zookeeper;
-
-import java.util.Objects;
-
-/**
- * A ZooKeeper server and its ID.
- *
- * @author mpolden
- */
-public class ZooKeeperServer {
-
-    private final int id;
-    private final String hostname;
-
-    public ZooKeeperServer(int id, String hostname) {
-        if (id < 0 || id > 255) throw new IllegalArgumentException("server id must be between 0 and 255");
-        this.id = id;
-        this.hostname = Objects.requireNonNull(hostname);
-    }
-
-    public int id() {
-        return id;
-    }
-
-    public String hostname() {
-        return hostname;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        ZooKeeperServer that = (ZooKeeperServer) o;
-        return id == that.id && hostname.equals(that.hostname);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(id, hostname);
-    }
-
-    @Override
-    public String toString() {
-        return "server " + id + "=" + hostname;
-    }
-}
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 97102ed7f58..94ccbc26f03 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
@@ -14,7 +14,6 @@ import java.io.IOException;
 import java.nio.file.Path;
 import java.time.Duration;
 import java.util.Arrays;
-import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -66,11 +65,6 @@ public class ReconfigurerTest {
         assertEquals(1, reconfigurer.reconfigurations());
         assertSame(nextConfig, reconfigurer.activeConfig());
 
-        // No reconfiguration happens with empty config
-        reconfigurer.startOrReconfigure(createConfig(0, true));
-        assertEquals(1, reconfigurer.reconfigurations());
-        assertSame(nextConfig, reconfigurer.activeConfig());
-
         // Cluster shrinks
         nextConfig = createConfig(3, true);
         reconfigurer.startOrReconfigure(nextConfig);
@@ -87,14 +81,6 @@ public class ReconfigurerTest {
         assertEquals("1=node2:2182:2183;2181,2=node3:2182:2183;2181", reconfigurer.joiningServers());
         assertEquals("1,2", reconfigurer.leavingServers());
         assertSame(nextConfig, reconfigurer.activeConfig());
-
-        // Reconfigure without using config
-        reconfigurer.reconfigure(List.of(new ZooKeeperServer(0, "node0"),
-                                         new ZooKeeperServer(1, "node2"),
-                                         new ZooKeeperServer(3, "node4")));
-        assertEquals(4, reconfigurer.reconfigurations());
-        assertEquals("3=node4:2182:2183;2181", reconfigurer.joiningServers());
-        assertEquals("2", reconfigurer.leavingServers());
     }
 
     @Test