Merge pull request #26638 from vespa-engine/jonmv/handle-majority-rejoining-zk-cluster

Compute joiners based on existing dynamic config

4 files changed, 175 insertions, 92 deletions

diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
index af42e30422b..0b2595b6af8 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
@@ -2,20 +2,28 @@
 package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.cloud.config.ZookeeperServerConfig;
+import com.yahoo.cloud.config.ZookeeperServerConfig.Server;
 import com.yahoo.security.tls.ConfigFileBasedTlsContext;
 import com.yahoo.security.tls.MixedMode;
 import com.yahoo.security.tls.TlsContext;
 import com.yahoo.security.tls.TransportSecurityUtils;
+
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
+import static com.yahoo.stream.CustomCollectors.toLinkedMap;
 import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 
 public class Configurator {
@@ -64,7 +72,6 @@ public class Configurator {
     // override of Vespa TLS config for unit testing
     void writeConfigToDisk(VespaTlsConfig vespaTlsConfig) {
         configFilePath.toFile().getParentFile().mkdirs();
-
         try {
             writeZooKeeperConfigFile(zookeeperServerConfig, vespaTlsConfig);
             writeMyIdFile(zookeeperServerConfig);
@@ -75,36 +82,75 @@ public class Configurator {
 
     private void writeZooKeeperConfigFile(ZookeeperServerConfig config,
                                           VespaTlsConfig vespaTlsConfig) throws IOException {
+        String dynamicConfigPath = config.dynamicReconfiguration() ? parseConfigFile(configFilePath).get("dynamicConfigFile") : null;
+        Map<String, String> dynamicConfig = dynamicConfigPath != null ? parseConfigFile(Paths.get(dynamicConfigPath)) : Map.of();
         try (FileWriter writer = new FileWriter(configFilePath.toFile())) {
-            writer.write(transformConfigToString(config, vespaTlsConfig));
+            writer.write(transformConfigToString(config, vespaTlsConfig, dynamicConfig));
         }
     }
 
-    private String transformConfigToString(ZookeeperServerConfig config, VespaTlsConfig vespaTlsConfig) {
-        StringBuilder sb = new StringBuilder();
-        sb.append("tickTime=").append(config.tickTime()).append("\n");
-        sb.append("initLimit=").append(config.initLimit()).append("\n");
-        sb.append("syncLimit=").append(config.syncLimit()).append("\n");
-        sb.append("maxClientCnxns=").append(config.maxClientConnections()).append("\n");
-        sb.append("snapCount=").append(config.snapshotCount()).append("\n");
-        sb.append("dataDir=").append(getDefaults().underVespaHome(config.dataDir())).append("\n");
-        sb.append("autopurge.purgeInterval=").append(config.autopurge().purgeInterval()).append("\n");
-        sb.append("autopurge.snapRetainCount=").append(config.autopurge().snapRetainCount()).append("\n");
+    private String transformConfigToString(ZookeeperServerConfig config, VespaTlsConfig vespaTlsConfig, Map<String, String> dynamicConfig) {
+        Map<String, String> configEntries = new LinkedHashMap<>();
+        configEntries.put("tickTime", Integer.toString(config.tickTime()));
+        configEntries.put("initLimit", Integer.toString(config.initLimit()));
+        configEntries.put("syncLimit", Integer.toString(config.syncLimit()));
+        configEntries.put("maxClientCnxns", Integer.toString(config.maxClientConnections()));
+        configEntries.put("snapCount", Integer.toString(config.snapshotCount()));
+        configEntries.put("dataDir", getDefaults().underVespaHome(config.dataDir()));
+        configEntries.put("autopurge.purgeInterval", Integer.toString(config.autopurge().purgeInterval()));
+        configEntries.put("autopurge.snapRetainCount", Integer.toString(config.autopurge().snapRetainCount()));
         // See http://zookeeper.apache.org/doc/r3.6.3/zookeeperAdmin.html#sc_zkCommands
         // Includes all available commands in 3.6, except 'wchc' and 'wchp'
-        sb.append("4lw.commands.whitelist=conf,cons,crst,dirs,dump,envi,mntr,ruok,srst,srvr,stat,wchs").append("\n");
-        sb.append("admin.enableServer=false").append("\n");
+        configEntries.put("4lw.commands.whitelist", "conf,cons,crst,dirs,dump,envi,mntr,ruok,srst,srvr,stat,wchs");
+        configEntries.put("admin.enableServer", "false");
         // Use custom connection factory for TLS on client port - see class' Javadoc for rationale
-        sb.append("serverCnxnFactory=org.apache.zookeeper.server.VespaNettyServerCnxnFactory").append("\n");
-        sb.append("quorumListenOnAllIPs=true").append("\n");
-        sb.append("standaloneEnabled=false").append("\n");
-        sb.append("reconfigEnabled=").append(config.dynamicReconfiguration()).append("\n");
-        sb.append("skipACL=yes").append("\n");
-        ensureThisServerIsRepresented(config.myid(), config.server());
-        config.server().forEach(server -> sb.append(serverSpec(server, server.joining())).append("\n"));
-        sb.append(new TlsQuorumConfig().createConfig(vespaTlsConfig));
-        sb.append(new TlsClientServerConfig().createConfig(vespaTlsConfig));
-        return sb.toString();
+        configEntries.put("serverCnxnFactory", "org.apache.zookeeper.server.VespaNettyServerCnxnFactory");
+        configEntries.put("quorumListenOnAllIPs", "true");
+        configEntries.put("standaloneEnabled", "false");
+        configEntries.put("reconfigEnabled", Boolean.toString(config.dynamicReconfiguration()));
+        configEntries.put("skipACL", "yes");
+
+        addServerSpecs(configEntries, config, dynamicConfig);
+
+        new TlsQuorumConfig().createConfig(configEntries, vespaTlsConfig);
+        new TlsClientServerConfig().createConfig(configEntries, vespaTlsConfig);
+        return transformConfigToString(configEntries);
+    }
+
+    void addServerSpecs(Map<String, String> configEntries, ZookeeperServerConfig config, Map<String, String> dynamicConfig) {
+        int myIndex = ensureThisServerIsRepresented(config.myid(), config.server());
+
+        // If dynamic config refers to servers that are not in the current config, we must ignore it.
+        Set<String> currentServers = config.server().stream().map(Server::hostname).collect(Collectors.toSet());
+        if (dynamicConfig.values().stream().anyMatch(spec -> ! currentServers.contains(spec.split(":", 2)[0]))) {
+            log.log(Level.WARNING, "Existing dynamic config refers to unknown servers, ignoring it");
+            dynamicConfig = Map.of();
+        }
+
+        // If we have no existing, valid, dynamic config, we use all known servers as a starting point.
+        if (dynamicConfig.isEmpty()) {
+            configEntries.putAll(getServerConfig(config.server(), config.server(myIndex).joining() ? config.myid() : -1));
+        }
+        // Otherwise, we use the existing, dynamic config as a starting point, and add this as a joiner if not present.
+        else {
+            Map.Entry<String, String> thisAsAJoiner = getServerConfig(config.server().subList(myIndex, myIndex + 1), config.myid()).entrySet().iterator().next();
+            dynamicConfig.putIfAbsent(thisAsAJoiner.getKey(), thisAsAJoiner.getValue());
+            configEntries.putAll(dynamicConfig);
+        }
+
+    }
+    static Map<String, String> getServerConfig(List<ZookeeperServerConfig.Server> serversConfig, int joinerId) {
+        Map<String, String> configEntries = new LinkedHashMap<>();
+        for (Server server : serversConfig) {
+            configEntries.put("server." + server.id(), serverSpec(server, server.id() == joinerId));
+        }
+        return configEntries;
+    }
+
+    static String transformConfigToString(Map<String, String> config) {
+        return config.entrySet().stream()
+                .map(entry -> entry.getKey() + "=" + entry.getValue())
+                .collect(Collectors.joining("\n", "", "\n"));
     }
 
     private void writeMyIdFile(ZookeeperServerConfig config) throws IOException {
@@ -113,25 +159,17 @@ public class Configurator {
         }
     }
 
-    private void ensureThisServerIsRepresented(int myid, List<ZookeeperServerConfig.Server> servers) {
-        boolean found = false;
-        for (ZookeeperServerConfig.Server server : servers) {
-            if (myid == server.id()) {
-                found = true;
-                break;
-            }
-        }
-        if (!found) {
-            throw new RuntimeException("No id in zookeeper server list that corresponds to my id (" + myid + ")");
+    private static int ensureThisServerIsRepresented(int myid, List<ZookeeperServerConfig.Server> servers) {
+        for (int i = 0; i < servers.size(); i++) {
+            Server server = servers.get(i);
+            if (myid == server.id()) return i;
         }
+        throw new RuntimeException("No id in zookeeper server list that corresponds to my id (" + myid + ")");
     }
 
     static String serverSpec(ZookeeperServerConfig.Server server, boolean joining) {
         StringBuilder sb = new StringBuilder();
-        sb.append("server.")
-          .append(server.id())
-          .append("=")
-          .append(server.hostname())
+        sb.append(server.hostname())
           .append(":")
           .append(server.quorumPort())
           .append(":")
@@ -150,6 +188,19 @@ public class Configurator {
         return sb.toString();
     }
 
+    static Map<String, String> parseConfigFile(Path configFilePath) {
+        try {
+            return Files.exists(configFilePath) ? Files.readAllLines(configFilePath).stream()
+                                                       .filter(line -> ! line.startsWith("#"))
+                                                       .map(line -> line.split("=", 2))
+                                                       .collect(toLinkedMap(parts -> parts[0], parts -> parts[1]))
+                                                : Map.of();
+        }
+        catch (IOException e) {
+            throw new UncheckedIOException("error reading zookeeper config", e);
+        }
+    }
+
     static List<String> zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) {
         return zookeeperServerConfig.server().stream()
                                     .map(ZookeeperServerConfig.Server::hostname)
@@ -165,15 +216,15 @@ public class Configurator {
     private interface TlsConfig {
         String configFieldPrefix();
 
-        default void appendSharedTlsConfig(StringBuilder builder, VespaTlsConfig vespaTlsConfig) {
+        default void appendSharedTlsConfig(Map<String, String> configEntries, VespaTlsConfig vespaTlsConfig) {
             vespaTlsConfig.context().ifPresent(ctx -> {
                 VespaSslContextProvider.set(ctx);
-                builder.append(configFieldPrefix()).append(".context.supplier.class=").append(VespaSslContextProvider.class.getName()).append("\n");
+                configEntries.put(configFieldPrefix() + ".context.supplier.class", VespaSslContextProvider.class.getName());
                 String enabledCiphers = Arrays.stream(ctx.parameters().getCipherSuites()).sorted().collect(Collectors.joining(","));
-                builder.append(configFieldPrefix()).append(".ciphersuites=").append(enabledCiphers).append("\n");
+                configEntries.put(configFieldPrefix() + ".ciphersuites", enabledCiphers);
                 String enabledProtocols = Arrays.stream(ctx.parameters().getProtocols()).sorted().collect(Collectors.joining(","));
-                builder.append(configFieldPrefix()).append(".enabledProtocols=").append(enabledProtocols).append("\n");
-                builder.append(configFieldPrefix()).append(".clientAuth=NEED\n");
+                configEntries.put(configFieldPrefix() + ".enabledProtocols", enabledProtocols);
+                configEntries.put(configFieldPrefix() + ".clientAuth", "NEED");
             });
         }
 
@@ -185,16 +236,13 @@ public class Configurator {
 
     static class TlsClientServerConfig implements TlsConfig {
 
-        public String createConfig(VespaTlsConfig vespaTlsConfig) {
-            StringBuilder sb = new StringBuilder()
-                    .append("client.portUnification=").append(enablePortUnification(vespaTlsConfig)).append("\n");
+        public void createConfig(Map<String, String> configEntries, VespaTlsConfig vespaTlsConfig) {
+            configEntries.put("client.portUnification", String.valueOf(enablePortUnification(vespaTlsConfig)));
             // ZooKeeper Dynamic Reconfiguration requires the "non-secure" client port to exist
             // This is a hack to override the secure parameter through our connection factory wrapper
             // https://issues.apache.org/jira/browse/ZOOKEEPER-3577
             VespaNettyServerCnxnFactory_isSecure = vespaTlsConfig.tlsEnabled() && vespaTlsConfig.mixedMode() == MixedMode.DISABLED;
-            appendSharedTlsConfig(sb, vespaTlsConfig);
-
-            return sb.toString();
+            appendSharedTlsConfig(configEntries, vespaTlsConfig);
         }
 
         @Override
@@ -205,12 +253,10 @@ public class Configurator {
 
     static class TlsQuorumConfig implements TlsConfig {
 
-        public String createConfig(VespaTlsConfig vespaTlsConfig) {
-            StringBuilder sb = new StringBuilder()
-                    .append("sslQuorum=").append(vespaTlsConfig.tlsEnabled()).append("\n")
-                    .append("portUnification=").append(enablePortUnification(vespaTlsConfig)).append("\n");
-            appendSharedTlsConfig(sb, vespaTlsConfig);
-            return sb.toString();
+        public void createConfig(Map<String, String> configEntries, VespaTlsConfig vespaTlsConfig) {
+            configEntries.put("sslQuorum", String.valueOf(vespaTlsConfig.tlsEnabled()));
+            configEntries.put("portUnification", String.valueOf(enablePortUnification(vespaTlsConfig)));
+            appendSharedTlsConfig(configEntries, vespaTlsConfig);
         }
 
         @Override
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 eb84b13d4d6..15431550d82 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
@@ -6,15 +6,15 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.annotation.Inject;
 import com.yahoo.protect.Process;
 import com.yahoo.yolean.Exceptions;
+
 import java.time.Duration;
 import java.time.Instant;
-import java.util.List;
 import java.util.Objects;
 import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static com.yahoo.vespa.zookeeper.Configurator.serverSpec;
+import static java.util.stream.Collectors.joining;
 
 /**
  * Starts zookeeper server and supports reconfiguring zookeeper cluster. Keep this as a component
@@ -26,9 +26,11 @@ public class Reconfigurer extends AbstractComponent {
 
     private static final Logger log = java.util.logging.Logger.getLogger(Reconfigurer.class.getName());
 
-    private static final Duration TIMEOUT = Duration.ofMinutes(15);
+    static final Duration TIMEOUT = Duration.ofMinutes(15);
 
     private final ExponentialBackoff backoff = new ExponentialBackoff(Duration.ofMillis(50), Duration.ofSeconds(10));
+    private final Duration timeout;
+    private final boolean haltOnFailure;
     private final VespaZooKeeperAdmin vespaZooKeeperAdmin;
     private final Sleeper sleeper;
 
@@ -38,12 +40,14 @@ public class Reconfigurer extends AbstractComponent {
 
     @Inject
     public Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin) {
-        this(vespaZooKeeperAdmin, new Sleeper());
+        this(vespaZooKeeperAdmin, new Sleeper(), true, TIMEOUT);
     }
 
-    Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin, Sleeper sleeper) {
+    public Reconfigurer(VespaZooKeeperAdmin vespaZooKeeperAdmin, Sleeper sleeper, boolean haltOnFailure, Duration timeout) {
         this.vespaZooKeeperAdmin = Objects.requireNonNull(vespaZooKeeperAdmin);
         this.sleeper = Objects.requireNonNull(sleeper);
+        this.haltOnFailure = haltOnFailure;
+        this.timeout = timeout;
     }
 
     @Override
@@ -86,14 +90,15 @@ public class Reconfigurer extends AbstractComponent {
     // TODO jonmv: wrap Curator in Provider, for Curator shutdown
     private void reconfigure(ZookeeperServerConfig newConfig) {
         Instant reconfigTriggered = Instant.now();
-        String newServers = String.join(",", servers(newConfig));
+        String newServers = servers(newConfig);
         log.log(Level.INFO, "Will reconfigure ZooKeeper cluster." +
                             "\nServers in active config:" + servers(activeConfig) +
-                            "\nServers in new config:" + servers(newConfig));
+                            "\nServers in new config:" + newServers);
         String connectionSpec = vespaZooKeeperAdmin.localConnectionSpec(activeConfig);
         Instant now = Instant.now();
-        Duration reconfigTimeout = reconfigTimeout();
-        Instant end = now.plus(reconfigTimeout);
+        // For reconfig to succeed, the current and resulting ensembles must have a majority. When an ensemble grows and
+        // the joining servers outnumber the existing ones, we have to wait for enough of them to start to have a majority.
+        Instant end = now.plus(timeout);
         // Loop reconfiguring since we might need to wait until another reconfiguration is finished before we can succeed
         for (int attempt = 1; ; attempt++) {
             try {
@@ -116,29 +121,20 @@ public class Reconfigurer extends AbstractComponent {
                 }
                 else {
                     log.log(Level.SEVERE, "Reconfiguration attempt " + attempt + " failed, and was failing for " +
-                                          reconfigTimeout + "; giving up now: " + Exceptions.toMessageString(e));
-                    shutdownAndDie(reconfigTimeout);
+                                          timeout + "; giving up now: " + Exceptions.toMessageString(e));
+                    shutdown();
+                    if (haltOnFailure)
+                        Process.logAndDie("Reconfiguration did not complete within timeout " + timeout + ". Forcing container shutdown.");
+                    else
+                        throw e;
                 }
             }
         }
     }
 
-    private void shutdownAndDie(Duration reconfigTimeout) {
-        shutdown();
-        Process.logAndDie("Reconfiguration did not complete within timeout " + reconfigTimeout + ". Forcing container shutdown.");
-    }
-
-    private static Duration reconfigTimeout() {
-        // For reconfig to succeed, the current and resulting ensembles must have a majority. When an ensemble grows and
-        // the joining servers outnumber the existing ones, we have to wait for enough of them to start to have a majority.
-        return TIMEOUT;
-    }
-
-    private static List<String> servers(ZookeeperServerConfig config) {
-        return config.server().stream()
-                     .filter(server -> ! server.retired())
-                     .map(server -> serverSpec(server, false))
-                     .toList();
+    private static String servers(ZookeeperServerConfig config) {
+        return Configurator.getServerConfig(config.server().stream().filter(server -> ! server.retired()).toList(), -1)
+                           .entrySet().stream().map(entry -> entry.getKey() + "=" + entry.getValue()).collect(joining(","));
     }
 
 }
diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
index 5d0031d5b55..08acbf2b838 100644
--- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
+++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
@@ -24,7 +24,10 @@ import java.math.BigInteger;
 import java.nio.file.Files;
 import java.security.KeyPair;
 import java.security.cert.X509Certificate;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import static com.yahoo.security.KeyAlgorithm.EC;
@@ -62,12 +65,12 @@ public class ConfiguratorTest {
     }
 
     @Test
-    public void config_is_written_correctly_with_multiple_servers() {
+    public void config_is_written_correctly_with_multiple_servers() throws IOException {
         three_config_servers(false);
     }
 
     @Test
-    public void config_is_written_correctly_with_multiple_servers_on_hosted_vespa() {
+    public void config_is_written_correctly_with_multiple_servers_on_hosted_vespa() throws IOException {
         three_config_servers(true);
     }
 
@@ -117,13 +120,49 @@ public class ConfiguratorTest {
         assertEquals("" + max_buffer, System.getProperty(ZOOKEEPER_JUTE_MAX_BUFFER));
     }
 
-    private void three_config_servers(boolean hosted) {
+    @Test
+    public void test_parsing_config() throws IOException {
         ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
         builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
         builder.server(newServer(0, "foo", 123, 321, false));
         builder.server(newServer(1, "bar", 234, 432, false));
         builder.server(newServer(2, "baz", 345, 543, true));
         builder.myidFile(idFile.getAbsolutePath());
+        builder.myid(2);
+        builder.tickTime(1234);
+        builder.dynamicReconfiguration(true);
+        Configurator configurator = new Configurator(builder.build());
+        configurator.writeConfigToDisk(VespaTlsConfig.tlsDisabled());
+        validateIdFile(idFile, "2\n");
+
+        assertEquals(Files.readString(cfgFile.toPath()),
+                     Configurator.transformConfigToString(Configurator.parseConfigFile(cfgFile.toPath())));
+
+        Map<String, String> originalConfig = Configurator.parseConfigFile(cfgFile.toPath());
+        Map<String, String> staticConfig = new LinkedHashMap<>(originalConfig);
+        // Dynamic config says this is not a joiner.
+        Map<String, String> dynamicConfig = Configurator.getServerConfig(builder.build().server(), -1);
+        staticConfig.keySet().removeAll(dynamicConfig.keySet());
+        assertEquals(originalConfig.size(), dynamicConfig.size() + staticConfig.size());
+        File dynFile = folder.newFile();
+        staticConfig.put("dynamicConfigFile", dynFile.getAbsolutePath());
+        Files.write(cfgFile.toPath(), Configurator.transformConfigToString(staticConfig).getBytes());
+        Files.write(dynFile.toPath(), Configurator.transformConfigToString(dynamicConfig).getBytes());
+
+        configurator.writeConfigToDisk(VespaTlsConfig.tlsDisabled());
+        // Next generation of config should not mark this as a joiner either.
+        originalConfig.putAll(Configurator.getServerConfig(builder.build().server().subList(2, 3), -1));
+        assertEquals(Configurator.transformConfigToString(originalConfig),
+                     Files.readString(cfgFile.toPath()));
+    }
+
+    private void three_config_servers(boolean hosted) throws IOException {
+        ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
+        builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
+        builder.server(newServer(0, "foo", 123, 321, false));
+        builder.server(newServer(1, "bar", 234, 432, true));
+        builder.server(newServer(2, "baz", 345, 543, false));
+        builder.myidFile(idFile.getAbsolutePath());
         builder.myid(1);
         builder.tickTime(1234);
         builder.dynamicReconfiguration(hosted);
@@ -205,8 +244,8 @@ public class ConfiguratorTest {
         String expected =
                 commonConfig(hosted) +
                 "server.0=foo:321:123;2181\n" +
-                "server.1=bar:432:234;2181\n" +
-                "server.2=baz:543:345:observer;2181\n" +
+                "server.1=bar:432:234:observer;2181\n" +
+                "server.2=baz:543:345;2181\n" +
                 "sslQuorum=false\n" +
                 "portUnification=false\n" +
                 "client.portUnification=false\n";
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 697fba3b4c4..ea8dcac945c 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
@@ -144,11 +144,13 @@ public class ReconfigurerTest {
 
         TestableReconfigurer(TestableVespaZooKeeperAdmin zooKeeperAdmin) {
             super(zooKeeperAdmin, new Sleeper() {
-                @Override
-                public void sleep(Duration duration) {
-                    // Do nothing
-                }
-            });
+                      @Override
+                      public void sleep(Duration duration) {
+                          // Do nothing
+                      }
+                  },
+                  false,
+                  Reconfigurer.TIMEOUT);
             this.zooKeeperAdmin = zooKeeperAdmin;
             HostName.setHostNameForTestingOnly("node1");
         }