diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-10-18 10:52:24 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2022-01-11 10:31:10 +0100 |
commit | e3d4838bf5cdd6e058b56b6e424f34ffe958d4ff (patch) | |
tree | 40f5cc89138060e0b13497f0386de24d07a76b67 /zookeeper-server/zookeeper-server-common/src | |
parent | 38a353a10345b2a5e17d241057bdbf662e4b05d1 (diff) |
Always reconfigure (idempotent), and to only non-retired servers
Diffstat (limited to 'zookeeper-server/zookeeper-server-common/src')
2 files changed, 21 insertions, 25 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 514d45c7e77..8b46c67e0f6 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 @@ -57,7 +57,7 @@ public class Reconfigurer extends AbstractComponent { zooKeeperRunner = startServer(newConfig, server); } - if (shouldReconfigure(newConfig)) { + if (newConfig.dynamicReconfiguration()) { reconfigure(newConfig); } return peer; @@ -73,23 +73,14 @@ public class Reconfigurer extends AbstractComponent { } } - private boolean shouldReconfigure(ZookeeperServerConfig newConfig) { - if (!newConfig.dynamicReconfiguration()) return false; - if (activeConfig == null) return false; - return ! newConfig.equals(activeConfig()); - } - private ZooKeeperRunner startServer(ZookeeperServerConfig zookeeperServerConfig, VespaZooKeeperServer server) { ZooKeeperRunner runner = new ZooKeeperRunner(zookeeperServerConfig, server); activeConfig = zookeeperServerConfig; return runner; } - // TODO jonmv: unify server config writing here and in Configurator // TODO jonmv: read dynamic file, discard if old quorum impossible (config file + .dynamic.<id>) // TODO jonmv: if dynamic file, all unlisted servers are observers; otherwise joiners are observers - // TODO jonmv: use bulk mode, i.e., supply only new servers - // TODO jonmv: always reconfigure when dynamic config is used, and to only non-retired nodes // TODO jonmv: verify reconfig by issuing a dummy write // TODO jonmv: wrap Curator in Provider, for Curator shutdown // TODO jonmv: scale down to 1 server as well @@ -153,6 +144,7 @@ public class Reconfigurer extends AbstractComponent { private static List<String> servers(ZookeeperServerConfig config) { return config.server().stream() + .filter(server -> ! server.retired()) .map(server -> serverSpec(server, config.clientPort(), false)) .collect(toList()); } 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 b868fd65de3..bafc81fdca1 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 @@ -53,18 +53,13 @@ public class ReconfigurerTest { 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(1, reconfigurer.reconfigurations()); - assertSame(nextConfig, reconfigurer.activeConfig()); - - // No reconfiguration happens with same config - reconfigurer.startOrReconfigure(nextConfig); - assertEquals(1, reconfigurer.reconfigurations()); + assertEquals(2, reconfigurer.reconfigurations()); assertSame(nextConfig, reconfigurer.activeConfig()); // Cluster shrinks nextConfig = createConfig(3, true); reconfigurer.startOrReconfigure(nextConfig); - assertEquals(2, reconfigurer.reconfigurations()); + assertEquals(3, 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()); @@ -73,7 +68,7 @@ public class ReconfigurerTest { // Cluster loses node1, but node3 joins. Indices are shuffled. nextConfig = createConfig(3, true, 1); reconfigurer.startOrReconfigure(nextConfig); - assertEquals(3, reconfigurer.reconfigurations()); + 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()); assertSame(nextConfig, reconfigurer.activeConfig()); @@ -91,7 +86,7 @@ public class ReconfigurerTest { 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(1, reconfigurer.reconfigurations()); + assertEquals(2, reconfigurer.reconfigurations()); assertSame(nextConfig, reconfigurer.activeConfig()); } @@ -112,24 +107,27 @@ public class ReconfigurerTest { reconfigurer.shutdown(); } - private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration, int... skipIndices) { - Arrays.sort(skipIndices); + private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration, int... retiredIndices) { + Arrays.sort(retiredIndices); ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); builder.myidFile(idFile.getAbsolutePath()); for (int i = 0, index = 0; i < numberOfServers; i++, index++) { - while (Arrays.binarySearch(skipIndices, index) >= 0) index++; - builder.server(newServer(i, "node" + index)); + boolean retired = Arrays.binarySearch(retiredIndices, index) >= 0; + if (retired) i--; + builder.server(newServer(i, "node" + index, retired)); } + builder.myid(0); builder.dynamicReconfiguration(dynamicReconfiguration); return builder.build(); } - private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName) { + private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName, boolean retired) { ZookeeperServerConfig.Server.Builder builder = new ZookeeperServerConfig.Server.Builder(); builder.id(id); builder.hostname(hostName); + builder.retired(retired); return builder; } @@ -142,6 +140,7 @@ 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) { @@ -157,6 +156,7 @@ public class ReconfigurerTest { void startOrReconfigure(ZookeeperServerConfig newConfig) { serverPeer = startOrReconfigure(newConfig, this, MockQuorumPeer::new); + phaser.arriveAndDeregister(); } String connectionSpec() { @@ -173,10 +173,14 @@ public class ReconfigurerTest { @Override public void shutdown() { + phaser.arriveAndAwaitAdvance(); serverPeer.shutdown(Duration.ofSeconds(1)); } @Override - public void start(Path configFilePath) { serverPeer.start(configFilePath); } + public void start(Path configFilePath) { + phaser.arriveAndAwaitAdvance(); + serverPeer.start(configFilePath); + } @Override public boolean reconfigurable() { |