Merge pull request #4504 from vespa-engine/hmusum/order-zookeeper-server-list-so-localhost-comes-first

Hmusum/order zookeeper server list so localhost comes first

2 files changed, 36 insertions, 11 deletions

diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
index 15257e11cbe..a9d099fea31 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.curator;
 
 import com.google.inject.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
+import com.yahoo.net.HostName;
 import com.yahoo.path.Path;
 import com.yahoo.vespa.curator.recipes.CuratorCounter;
 import com.yahoo.vespa.zookeeper.ZooKeeperServer;
@@ -21,8 +22,8 @@ import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.framework.state.ConnectionStateListener;
 import org.apache.curator.retry.ExponentialBackoffRetry;
 
-import java.io.Closeable;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -69,17 +70,26 @@ public class Curator implements AutoCloseable {
     }
     
     private static String createConnectionSpec(ConfigserverConfig config) {
-        StringBuilder sb = new StringBuilder();
-        for (int i = 0; i < config.zookeeperserver().size(); i++) {
+        List<String> servers = createAndOrderServerList(config);
+        return String.join(",", servers);
+    }
+
+    private static List<String> createAndOrderServerList(ConfigserverConfig config) {
+        String hostName = HostName.getLocalhost();
+        int configServerCount = config.zookeeperserver().size();
+        List<String> servers = new ArrayList<>();
+
+        int indexForThisConfigServer = 0;
+        for (int i = 0; i < configServerCount; i++) {
             ConfigserverConfig.Zookeeperserver server = config.zookeeperserver(i);
-            sb.append(server.hostname());
-            sb.append(":");
-            sb.append(server.port());
-            if (i < config.zookeeperserver().size() - 1) {
-                sb.append(",");
-            }
+            servers.add(server.hostname() + ":" + server.port());
+            if (server.hostname().equals(hostName))
+                indexForThisConfigServer = i;
         }
-        return sb.toString();
+        // Rotate list of servers so that config server on this host is first in the list
+        if (indexForThisConfigServer != 0)
+            Collections.rotate(servers, configServerCount - indexForThisConfigServer);
+        return servers;
     }
 
     /** 
diff --git a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/CuratorTest.java b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/CuratorTest.java
index 36205bdaca3..396555d5e3a 100644
--- a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/CuratorTest.java
+++ b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/CuratorTest.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
+import com.yahoo.net.HostName;
 import com.yahoo.vespa.curator.Curator;
 import org.apache.curator.test.TestingServer;
 import org.junit.After;
@@ -11,8 +12,8 @@ import org.junit.Test;
 import java.io.IOException;
 
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Sets up actual ZooKeeper servers and verifies we can talk to them.
@@ -74,6 +75,20 @@ public class CuratorTest {
         }
     }
 
+    @Test
+    public void require_that_server_order_is_correct() {
+        ConfigserverConfig.Builder builder = new ConfigserverConfig.Builder();
+        builder.zookeeperserver(createZKBuilder("localhost", port1));
+        builder.zookeeperserver(createZKBuilder(HostName.getLocalhost(), port2));
+        builder.zookeeperserver(createZKBuilder("localhost", 1234));
+        builder.zookeeperserver(createZKBuilder("localhost", 6789));
+        try (Curator curator = createCurator(new ConfigserverConfig(builder))) {
+            assertThat(curator.serverCount(), is(4));
+            // host this is running on should come first
+            assertTrue(curator.connectionSpec().startsWith(HostName.getLocalhost() + ":" + port2 + ",localhost:1234"));
+        }
+    }
+
     private ConfigserverConfig createTestConfig() {
         ConfigserverConfig.Builder builder = new ConfigserverConfig.Builder();
         builder.zookeeperserver(createZKBuilder("localhost", port1));