Code cleanup (no functional changes)

4 files changed, 22 insertions, 26 deletions

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 75020b23607..a27b33173ee 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
@@ -291,10 +291,15 @@ public class HostResource implements Comparable<HostResource> {
             return this.getHostname().compareTo(other.getHostname());
     }
 
+    public static List<HostResource> pickHosts(List<HostResource> hostsSelectedByIndex, int count) {
+        return hostsSelectedByIndex.subList(0, Math.min(count, hostsSelectedByIndex.size()));
+    }
+
     /**
      * Picks hosts by some mixture of host name and index 
      * (where the mix of one or the other is decided by the last parameter).
      */
+    // TODO: Use pickHosts with 2 arguments (above) instead of this
     public static List<HostResource> pickHosts(Collection<HostResource> hosts, int count, int targetHostsSelectedByIndex) {
         targetHostsSelectedByIndex = Math.min(Math.min(targetHostsSelectedByIndex, count), hosts.size());
 
@@ -303,17 +308,14 @@ public class HostResource implements Comparable<HostResource> {
 
         List<HostResource> hostsSortedByIndex = new ArrayList<>(hosts);
         hostsSortedByIndex.sort((a, b) -> a.comparePrimarilyByIndexTo(b));
-        return pickHosts(hostsSortedByName, hostsSortedByIndex, count, targetHostsSelectedByIndex);
-    }
-    public static List<HostResource> pickHosts(List<HostResource> hostsSelectedByName, List<HostResource> hostsSelectedByIndex, 
-                                               int count, int targetHostsSelectedByIndex) {
-        hostsSelectedByName = hostsSelectedByName.subList(0, Math.min(count - targetHostsSelectedByIndex, hostsSelectedByName.size()));
-        hostsSelectedByIndex.removeAll(hostsSelectedByName);
-        hostsSelectedByIndex = hostsSelectedByIndex.subList(0, Math.min(targetHostsSelectedByIndex, hostsSelectedByIndex.size()));
+
+        hostsSortedByName = hostsSortedByName.subList(0, Math.min(count - targetHostsSelectedByIndex, hostsSortedByName.size()));
+        hostsSortedByIndex.removeAll(hostsSortedByName);
+        hostsSortedByIndex = hostsSortedByIndex.subList(0, Math.min(targetHostsSelectedByIndex, hostsSortedByIndex.size()));
 
         List<HostResource> finalHosts = new ArrayList<>();
-        finalHosts.addAll(hostsSelectedByName);
-        finalHosts.addAll(hostsSelectedByIndex);
+        finalHosts.addAll(hostsSortedByName);
+        finalHosts.addAll(hostsSortedByIndex);
         return finalHosts;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
index c07d6f66237..2c23043abea 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
@@ -155,9 +155,8 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         Collection<ContainerModel> containerModelsWithSlobrok = containerModels.stream()
                 .filter(this::shouldHaveSlobrok)
                 .collect(Collectors.toList());
-        int hostsPerCluster = (int) Math.max(
-                minHostsPerContainerCluster,
-                Math.ceil((double) count / containerModelsWithSlobrok.size()));
+        int hostsPerCluster = (int) Math.max(minHostsPerContainerCluster,
+                                             Math.ceil((double) count / containerModelsWithSlobrok.size()));
 
         // Pick from all container clusters to make sure we don't lose all nodes at once if some clusters are removed.
         // This will overshoot the desired size (due to ceil and picking at least one node per cluster).
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 30e5962b3b3..fb3af7f3652 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
@@ -352,11 +352,9 @@ public class ContentCluster extends AbstractConfigProducer implements
         }
 
         private List<HostResource> drawControllerHosts(int count, StorageGroup rootGroup, Collection<ContainerModel> containers) {
-            List<HostResource> hostsByName = drawContentHostsRecursively(count, false, rootGroup);
-            List<HostResource> hostsByIndex = drawContentHostsRecursively(count, true, rootGroup);
+            List<HostResource> hosts = drawContentHostsRecursively(count, rootGroup);
             // if (hosts.size() < count) // supply with containers TODO: Currently disabled due to leading to topology change problems
             //     hosts.addAll(drawContainerHosts(count - hosts.size(), containers, new HashSet<>(hosts)));
-            List<HostResource> hosts = HostResource.pickHosts(hostsByName, hostsByIndex, count, 3);
             if (hosts.size() % 2 == 0) // ZK clusters of even sizes are less available (even in the size=2 case)
                 hosts = hosts.subList(0, hosts.size()-1);
             return hosts;
@@ -429,12 +427,12 @@ public class ContentCluster extends AbstractConfigProducer implements
          */
         // Note: This method cannot be changed to draw different nodes without ensuring that it will draw nodes
         //       which overlaps with previously drawn nodes as this will prevent rolling upgrade
-        private List<HostResource> drawContentHostsRecursively(int count, boolean byIndex, StorageGroup group) {
+        private List<HostResource> drawContentHostsRecursively(int count, StorageGroup group) {
             Set<HostResource> hosts = new HashSet<>();
             if (group.getNodes().isEmpty()) {
                 int hostsPerSubgroup = (int)Math.ceil((double)count / group.getSubgroups().size());
                 for (StorageGroup subgroup : group.getSubgroups())
-                    hosts.addAll(drawContentHostsRecursively(hostsPerSubgroup, byIndex, subgroup));
+                    hosts.addAll(drawContentHostsRecursively(hostsPerSubgroup, subgroup));
             }
             else {
                 hosts.addAll(group.getNodes().stream()
@@ -443,10 +441,7 @@ public class ContentCluster extends AbstractConfigProducer implements
             }
 
             List<HostResource> sortedHosts = new ArrayList<>(hosts);
-            if (byIndex)
-                sortedHosts.sort((a, b) -> (a.comparePrimarilyByIndexTo(b)));
-            else // by name
-                Collections.sort(sortedHosts);
+            sortedHosts.sort((a, b) -> (a.comparePrimarilyByIndexTo(b)));
             sortedHosts = sortedHosts.subList(0, Math.min(count, hosts.size()));
             return sortedHosts;
         }
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index c9d694ec99f..3eb1d51a57e 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -565,11 +565,11 @@ public class ModelProvisioningTest {
         assertEquals( 8, cluster.distributionBits());
         assertEquals("We get the closest odd number", 5, clusterControllers.getContainers().size());
         assertEquals("bar-controllers", clusterControllers.getName());
-        assertEquals("default01", clusterControllers.getContainers().get(0).getHostName());
-        assertEquals("default02", clusterControllers.getContainers().get(1).getHostName());
-        assertEquals("default04", clusterControllers.getContainers().get(2).getHostName());
-        assertEquals("default09", clusterControllers.getContainers().get(3).getHostName());
-        assertEquals("default08", clusterControllers.getContainers().get(4).getHostName());
+        assertEquals("default09", clusterControllers.getContainers().get(0).getHostName());
+        assertEquals("default08", clusterControllers.getContainers().get(1).getHostName());
+        assertEquals("default06", clusterControllers.getContainers().get(2).getHostName());
+        assertEquals("default05", clusterControllers.getContainers().get(3).getHostName());
+        assertEquals("default03", clusterControllers.getContainers().get(4).getHostName());
         assertEquals("default09", cluster.getRootGroup().getSubgroups().get(0).getNodes().get(0).getHostName());
         assertEquals("default08", cluster.getRootGroup().getSubgroups().get(0).getNodes().get(1).getHostName());
         assertEquals("default06", cluster.getRootGroup().getSubgroups().get(1).getNodes().get(0).getHostName());