diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-02-17 15:32:25 +0100 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2021-02-17 15:32:25 +0100 |
commit | b69abc19d8f3f291639c2be9d21c28512177c231 (patch) | |
tree | 8f786fbb0e71974c5d54154a3af9215de8f3c91c /zookeeper-server/zookeeper-server-common | |
parent | 49adf2dd1f96f8e5636f5b3ee80fdf56c96aff86 (diff) |
Properly detect leaving ZK servers by comparing full spec
Diffstat (limited to 'zookeeper-server/zookeeper-server-common')
2 files changed, 26 insertions, 15 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 33191718bfb..4ef73ec2374 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 @@ -86,12 +86,12 @@ public class Reconfigurer extends AbstractComponent { if (newConfig.server().size() == 1) shutdownAndDie(server, Duration.ZERO); List<String> newServers = difference(servers(newConfig), servers(activeConfig)); - String leavingServers = String.join(",", difference(serverIds(activeConfig), serverIds(newConfig))); - String joiningServers = String.join(",", newServers); - leavingServers = leavingServers.isEmpty() ? null : leavingServers; - joiningServers = joiningServers.isEmpty() ? null : joiningServers; - log.log(Level.INFO, "Will reconfigure ZooKeeper cluster. \nJoining servers: " + joiningServers + - "\nleaving servers: " + leavingServers + + String leavingServerIds = String.join(",", serverIdsDifference(activeConfig, newConfig)); + String joiningServersSpec = String.join(",", newServers); + leavingServerIds = leavingServerIds.isEmpty() ? null : leavingServerIds; + joiningServersSpec = joiningServersSpec.isEmpty() ? null : joiningServersSpec; + log.log(Level.INFO, "Will reconfigure ZooKeeper cluster. \nJoining servers: " + joiningServersSpec + + "\nleaving servers: " + leavingServerIds + "\nServers in active config:" + servers(activeConfig) + "\nServers in new config:" + servers(newConfig)); String connectionSpec = localConnectionSpec(activeConfig); @@ -102,7 +102,7 @@ public class Reconfigurer extends AbstractComponent { for (int attempt = 1; now.isBefore(end); attempt++) { try { Instant reconfigStarted = Instant.now(); - vespaZooKeeperAdmin.reconfigure(connectionSpec, joiningServers, leavingServers); + vespaZooKeeperAdmin.reconfigure(connectionSpec, joiningServersSpec, leavingServerIds); Instant reconfigEnded = Instant.now(); log.log(Level.INFO, "Reconfiguration completed in " + Duration.between(reconfigTriggered, reconfigEnded) + @@ -141,11 +141,10 @@ public class Reconfigurer extends AbstractComponent { return HostName.getLocalhost() + ":" + config.clientPort(); } - private static List<String> serverIds(ZookeeperServerConfig config) { - return config.server().stream() - .map(ZookeeperServerConfig.Server::id) - .map(String::valueOf) - .collect(Collectors.toList()); + private static List<String> serverIdsDifference(ZookeeperServerConfig oldConfig, ZookeeperServerConfig newConfig) { + return difference(servers(oldConfig), servers(newConfig)).stream() + .map(server -> server.substring(0, server.indexOf('='))) + .collect(Collectors.toList()); } private static List<String> servers(ZookeeperServerConfig config) { 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 a1b98a23bd0..5cee0de2b6e 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 @@ -13,7 +13,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.time.Duration; -import java.util.stream.IntStream; +import java.util.Arrays; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -68,6 +68,14 @@ public class ReconfigurerTest { 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(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()); } @Test @@ -107,11 +115,15 @@ public class ReconfigurerTest { reconfigurer.shutdown(); } - private ZookeeperServerConfig createConfig(int numberOfServers, boolean dynamicReconfiguration) { + 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()); - IntStream.range(0, numberOfServers).forEach(i -> builder.server(newServer(i, "node" + i))); + for (int i = 0, index = 0; i < numberOfServers; i++, index++) { + while (Arrays.binarySearch(skipIndices, index) >= 0) index++; + builder.server(newServer(i, "node" + index)); + } builder.myid(0); builder.dynamicReconfiguration(dynamicReconfiguration); return builder.build(); |