Fall back to sort by hostname if a host has no membership

13 files changed, 45 insertions, 38 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java b/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java
index 30d83a32ff9..912968747df 100644
--- a/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java
+++ b/config-model/src/main/java/com/yahoo/config/model/ApplicationConfigProducerRoot.java
@@ -29,7 +29,6 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 
 
@@ -207,7 +206,7 @@ public class ApplicationConfigProducerRoot extends AbstractConfigProducer<Abstra
         builder.vespaVersion(vespaVersion.toSerializedForm());
         for (HostResource modelHost : getHostSystem().getHosts()) {
             builder.hosts(new Hosts.Builder()
-                    .name(modelHost.getHostName())
+                    .name(modelHost.getHostname())
                     .services(getServices(modelHost))
             );
         }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
index b4b89278c51..dfa2d2cefb0 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java
@@ -315,7 +315,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon
      * @return The hostname on which this service runs.
      */
     public String getHostName() {
-        return hostResource.getHostName();
+        return hostResource.getHostname();
     }
 
     /**
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/Host.java b/config-model/src/main/java/com/yahoo/vespa/model/Host.java
index e4e1e029a1a..0adfe9e4bdb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/Host.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/Host.java
@@ -67,7 +67,7 @@ public final class Host extends AbstractConfigProducer<AbstractConfigProducer<?>
         runsConfigServer = false;
     }
 
-    public String getHostName() {
+    public String getHostname() {
         return hostname;
     }
 
@@ -77,7 +77,7 @@ public final class Host extends AbstractConfigProducer<AbstractConfigProducer<?>
 
     /** Returns the string representation of this Host object. */
     public String toString() {
-        return "host '" + getHostName() + "'";
+        return "host '" + getHostname() + "'";
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java b/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java
index e6ed91165ca..72309d4c266 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java
@@ -207,7 +207,7 @@ public class HostResource implements Comparable<HostResource> {
     }
 
     public HostInfo getHostInfo() {
-        return new HostInfo(getHostName(), services.values().stream()
+        return new HostInfo(getHostname(), services.values().stream()
                 .map(Service::getServiceInfo)
                 .collect(Collectors.toSet()));
     }
@@ -244,11 +244,11 @@ public class HostResource implements Comparable<HostResource> {
 
     @Override
     public String toString() {
-        return "host '" + host.getHostName() + "'";
+        return "host '" + host.getHostname() + "'";
     }
 
-    public String getHostName() {
-        return host.getHostName();
+    public String getHostname() {
+        return host.getHostname();
     }
 
     @Override
@@ -267,6 +267,19 @@ public class HostResource implements Comparable<HostResource> {
     }
 
     /**
+     * Compares by the index of the primary membership, if both hosts are members in at least one cluster at this time.
+     * Compare by hostname otherwise.
+     */
+    public int comparePrimarilyByIndexTo(HostResource other) {
+        Optional<ClusterMembership> thisMembership = this.primaryClusterMembership();
+        Optional<ClusterMembership> otherMembership = other.primaryClusterMembership();
+        if (thisMembership.isPresent() && otherMembership.isPresent())
+            return Integer.compare(thisMembership.get().index(), otherMembership.get().index());
+        else
+            return this.getHostname().compareTo(other.getHostname());
+    }
+
+    /**
      * Picks hosts by some mixture of host name and index 
      * (where the mix of one or the other is decided by the last parameter).
      */
@@ -277,7 +290,7 @@ public class HostResource implements Comparable<HostResource> {
         Collections.sort(hostsSortedByName);
 
         List<HostResource> hostsSortedByIndex = new ArrayList<>(hosts);
-        hostsSortedByIndex.sort(Comparator.comparingInt(host -> host.primaryClusterMembership().get().index()));
+        hostsSortedByIndex.sort((a, b) -> a.comparePrimarilyByIndexTo(b));
         return pickHosts(hostsSortedByName, hostsSortedByIndex, count, targetHostsSelectedByIndex);
     }
     public static List<HostResource> pickHosts(List<HostResource> hostsSelectedByName, List<HostResource> hostsSelectedByIndex, 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java b/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java
index 2db9b6acece..6ef6c208c0a 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java
@@ -101,7 +101,7 @@ public class HostSystem extends AbstractConfigProducer<Host> {
     public HostResource getHost(String hostAlias) {
         HostSpec hostSpec = provisioner.allocateHost(hostAlias);
         for (HostResource resource : hostname2host.values()) {
-            if (resource.getHostName().equals(hostSpec.hostname())) {
+            if (resource.getHostname().equals(hostSpec.hostname())) {
                 hostSpec.membership().ifPresent(resource::addClusterMembership);
                 return resource;
             }
@@ -114,8 +114,8 @@ public class HostSystem extends AbstractConfigProducer<Host> {
         HostResource hostResource = new HostResource(host);
         hostResource.setFlavor(hostSpec.flavor());
         hostSpec.membership().ifPresent(hostResource::addClusterMembership);
-        hostname2host.put(host.getHostName(), hostResource);
-        log.log(DEBUG, () -> "Added new host resource for " + host.getHostName() + " with flavor " + hostResource.getFlavor());
+        hostname2host.put(host.getHostname(), hostResource);
+        log.log(DEBUG, () -> "Added new host resource for " + host.getHostname() + " with flavor " + hostResource.getFlavor());
         return hostResource;
     }
 
@@ -136,16 +136,16 @@ public class HostSystem extends AbstractConfigProducer<Host> {
             retAllocatedHosts.put(host, spec.membership().orElse(null));
             if (! host.getFlavor().isPresent()) {
                 host.setFlavor(spec.flavor());
-                log.log(DEBUG, () -> "Host resource " + host.getHostName() + " had no flavor, setting to " + spec.flavor());
+                log.log(DEBUG, () -> "Host resource " + host.getHostname() + " had no flavor, setting to " + spec.flavor());
             }
         }
-        retAllocatedHosts.keySet().forEach(host -> log.log(DEBUG, () -> "Allocated host " + host.getHostName() + " with flavor " + host.getFlavor()));
+        retAllocatedHosts.keySet().forEach(host -> log.log(DEBUG, () -> "Allocated host " + host.getHostname() + " with flavor " + host.getFlavor()));
         return retAllocatedHosts;
     }
 
     private Optional<HostResource> getExistingHost(HostSpec key) {
         List<HostResource> hosts = hostname2host.values().stream()
-                .filter(resource -> resource.getHostName().equals(key.hostname()))
+                .filter(resource -> resource.getHostname().equals(key.hostname()))
                 .collect(Collectors.toList());
         if (hosts.isEmpty()) {
             return Optional.empty();
@@ -156,12 +156,12 @@ public class HostSystem extends AbstractConfigProducer<Host> {
     }
 
     public void addBoundHost(HostResource host) {
-        hostname2host.put(host.getHostName(), host);
+        hostname2host.put(host.getHostname(), host);
     }
 
     Set<HostSpec> getHostSpecs() {
         return getHosts().stream()
-                .map(host -> new HostSpec(host.getHostName(), Collections.emptyList(),
+                .map(host -> new HostSpec(host.getHostname(), Collections.emptyList(),
                                           host.getFlavor(), host.primaryClusterMembership()))
                 .collect(Collectors.toCollection(LinkedHashSet::new));
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
index 69e9a0e8585..60d00ce5487 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
@@ -215,7 +215,7 @@ public class Admin extends AbstractConfigProducer implements Serializable {
                                        fileDistributor.fileSourceHost() + "'. Hostsystem=" + getHostSystem());
         }
 
-        FileDistributorService fds = new FileDistributorService(fileDistribution, host.getHost().getHostName(),
+        FileDistributorService fds = new FileDistributorService(fileDistribution, host.getHost().getHostname(),
             fileDistribution.getFileDistributor(), fileDistribution.getOptions(), host == deployHost);
         fds.setHostResource(host);
         fds.initService();
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 16b0674fc14..fc267133c9f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -7,10 +7,7 @@ import com.yahoo.config.model.ConfigModelContext;
 import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
-import com.yahoo.config.provision.RegionName;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
-import com.yahoo.lang.MutableInteger;
 import com.yahoo.vespa.config.content.MessagetyperouteselectorpolicyConfig;
 import com.yahoo.vespa.config.content.FleetcontrollerConfig;
 import com.yahoo.vespa.config.content.StorDistributionConfig;
@@ -360,7 +357,7 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
             // have one cluster controller
             List<HostResource> uniqueHostsWithoutClusterController = allHosts.stream()
                     .filter(h -> ! usedHosts.contains(h))
-                    .filter(h -> ! hostHasClusterController(h.getHostName(), allHosts))
+                    .filter(h -> ! hostHasClusterController(h.getHostname(), allHosts))
                     .distinct()
                     .collect(Collectors.toList());
 
@@ -384,7 +381,7 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
         /** Returns whether any host having the given hostname has a cluster controller */
         private boolean hostHasClusterController(String hostname, List<HostResource> hosts) {
             for (HostResource host : hosts) {
-                if ( ! host.getHostName().equals(hostname)) continue;
+                if ( ! host.getHostname().equals(hostname)) continue;
 
                 if (hasClusterController(host))
                     return true;
@@ -421,7 +418,7 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
 
             List<HostResource> sortedHosts = new ArrayList<>(hosts);
             if (byIndex)
-                sortedHosts.sort(Comparator.comparingInt(host -> host.primaryClusterMembership().get().index()));
+                sortedHosts.sort((a, b) -> (a.comparePrimarilyByIndexTo(b)));
             else // by name
                 Collections.sort(sortedHosts);
             sortedHosts = sortedHosts.subList(0, Math.min(count, hosts.size()));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java
index f6cc9203d00..443c88355ed 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java
@@ -9,8 +9,6 @@ import com.yahoo.vespa.model.Host;
 import java.util.*;
 import java.util.stream.Collectors;
 
-import static java.util.Arrays.asList;
-
 
 /**
  * Responsible for directing distribution of files to hosts.
@@ -81,7 +79,7 @@ public class FileDistributor {
     }
 
     public Set<String> getTargetHostnames() {
-        return getTargetHosts().stream().map(Host::getHostName).collect(Collectors.toSet());
+        return getTargetHosts().stream().map(Host::getHostname).collect(Collectors.toSet());
     }
 
     /** Returns the host which is the source of the files */
@@ -97,8 +95,8 @@ public class FileDistributor {
     public void sendDeployedFiles(FileDistribution dbHandler) {
         String fileSourceHost = fileSourceHost();
         for (Host host : getTargetHosts()) {
-            if ( ! host.getHostName().equals(fileSourceHost)) {
-                dbHandler.sendDeployedFiles(host.getHostName(), filesToSendToHost(host));
+            if ( ! host.getHostname().equals(fileSourceHost)) {
+                dbHandler.sendDeployedFiles(host.getHostname(), filesToSendToHost(host));
             }
         }
         dbHandler.sendDeployedFiles(fileSourceHost, allFilesToSend());
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java b/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java
index c0867d252a3..c7610e784ec 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java
@@ -97,7 +97,7 @@ public class Dispatch extends AbstractService implements SearchInterface,
     public int getMaxThreads() { return 500*2; }
 
     public String getHostname() {
-        return getHost().getHostName();
+        return getHost().getHostname();
     }
 
     @Override
@@ -106,7 +106,7 @@ public class Dispatch extends AbstractService implements SearchInterface,
     }
 
     public String getDispatcherConnectSpec() {
-        return "tcp/" + getHost().getHostName() + ":" + getDispatchPort();
+        return "tcp/" + getHost().getHostname() + ":" + getDispatchPort();
     }
 
     public DispatchGroup getDispatchGroup() {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java
index 8d7d289b99f..4925b88e608 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java
@@ -200,7 +200,7 @@ public abstract class IndexedSearchCluster extends SearchCluster
             String containerClusterName = containerCluster.getName();
             log.log(LogLevel.DEBUG, "Adding tld with index " + containerIndex + " for content cluster " + this.getClusterName() +
                                     ", container cluster " + containerClusterName + " (container id " + containerSubId +
-                                    ") on host " + container.getHostResource().getHostName());
+                                    ") on host " + container.getHostResource().getHostname());
             rootDispatch.addDispatcher(createTld(tldParent, container.getHostResource(), containerClusterName, containerIndex));
         }
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
index c5f7272ee80..05b0b382814 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java
@@ -183,7 +183,7 @@ public class SearchNode extends AbstractService implements
      * @return The connection string.
      */
     public String getDispatcherConnectSpec() {
-        return "tcp/" + getHost().getHostName() + ":" + getDispatchPort();
+        return "tcp/" + getHost().getHostname() + ":" + getDispatchPort();
     }
 
     public int getDispatchPort() {
@@ -245,7 +245,7 @@ public class SearchNode extends AbstractService implements
             partition(getNodeSpec().partitionId()).
             clustername(getClusterName()).
             basedir(getBaseDir()).
-            tlsspec("tcp/" + getHost().getHostName() + ":" + getTransactionLogServer().getTlsPort()).
+            tlsspec("tcp/" + getHost().getHostname() + ":" + getTransactionLogServer().getTlsPort()).
             tlsconfigid(getConfigId()).
             slobrokconfigid(getClusterConfigId()).
             routingconfigid(getClusterConfigId()).
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java
index 8bd62789d52..6c1bf6c9748 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java
@@ -124,7 +124,7 @@ public class DomAdminV2BuilderTest extends DomBuilderTest {
         assertThat(admin.getClusterControllerHosts().size(), is(1));
         assertNotNull(admin.getHostSystem().getHostByHostname("test1"));
         for (Configserver configserver : admin.getConfigservers()) {
-            assertThat(configserver.getHostName(), is(not(admin.getClusterControllerHosts().get(0).getHost().getHostName())));
+            assertThat(configserver.getHostName(), is(not(admin.getClusterControllerHosts().get(0).getHost().getHostname())));
             for (Slobrok slobrok : admin.getSlobroks()) {
                     assertThat(slobrok.getHostName(), is(not(configserver.getHostName())));
             }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilderTest.java
index 51e982f8a34..77cc881e0d9 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilderTest.java
@@ -122,9 +122,9 @@ public class VespaDomBuilderTest {
         System.out.println(hostSystem);
         assertThat(hostSystem.getHosts().size(), is(1));
         HostResource host = hostSystem.getHosts().get(0);
-        assertThat(host, is(hostSystem.getHostByHostname(host.getHostName())));
+        assertThat(host, is(hostSystem.getHostByHostname(host.getHostname())));
         assertNotNull(hostSystem.getHost("node1"));
-        assertThat(hostSystem.toString(), is("host '" + host.getHostName() + "'"));
+        assertThat(hostSystem.toString(), is("host '" + host.getHostname() + "'"));
     }
 
     private VespaModel createModel(String hosts, String services) {