From 152a346813cd0b5faa1b08ead18f8142d6b110e5 Mon Sep 17 00:00:00 2001 From: jonmv Date: Wed, 29 Mar 2023 19:51:35 +0200 Subject: Compute joiners based on existing dynamic config --- .../com/yahoo/vespa/zookeeper/Configurator.java | 156 +++++++++++++-------- .../com/yahoo/vespa/zookeeper/Reconfigurer.java | 50 +++---- .../yahoo/vespa/zookeeper/ConfiguratorTest.java | 49 ++++++- .../yahoo/vespa/zookeeper/ReconfigurerTest.java | 12 +- 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 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 dynamicConfig) { + Map 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 configEntries, ZookeeperServerConfig config, Map 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 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 thisAsAJoiner = getServerConfig(config.server().subList(myIndex, myIndex + 1), config.myid()).entrySet().iterator().next(); + dynamicConfig.putIfAbsent(thisAsAJoiner.getKey(), thisAsAJoiner.getValue()); + configEntries.putAll(dynamicConfig); + } + + } + static Map getServerConfig(List serversConfig, int joinerId) { + Map configEntries = new LinkedHashMap<>(); + for (Server server : serversConfig) { + configEntries.put("server." + server.id(), serverSpec(server, server.id() == joinerId)); + } + return configEntries; + } + + static String transformConfigToString(Map 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 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 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 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 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 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 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 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 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 originalConfig = Configurator.parseConfigFile(cfgFile.toPath()); + Map staticConfig = new LinkedHashMap<>(originalConfig); + // Dynamic config says this is not a joiner. + Map 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"); } -- cgit v1.2.3