Merge pull request #3116 from vespa-engine/hmusum/set-default-slobroks-per-container-cluster-to-2

Set default number of slobroks per container cluster to 2

3 files changed, 37 insertions, 65 deletions

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 7795e53832d..5e22ba3961f 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
@@ -49,17 +49,16 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         Optional<NodesSpecification> requestedLogservers = 
                 NodesSpecification.optionalDedicatedFromParent(adminElement.getChild("logservers"), version);
 
-        int minSlobroksPerCluster = getMinSlobroksPerContainerCluster(adminElement);
-        assignSlobroks(requestedSlobroks.orElse(NodesSpecification.nonDedicated(3, version)), admin, minSlobroksPerCluster);
+        assignSlobroks(requestedSlobroks.orElse(NodesSpecification.nonDedicated(3, version)), admin);
         assignLogserver(requestedLogservers.orElse(NodesSpecification.nonDedicated(1, version)), admin);
     }
 
-    private void assignSlobroks(NodesSpecification nodesSpecification, Admin admin, int minSlobroksPerContainerCluster) {
+    private void assignSlobroks(NodesSpecification nodesSpecification, Admin admin) {
         if (nodesSpecification.isDedicated()) {
             createSlobroks(admin, allocateHosts(admin.getHostSystem(), "slobroks", nodesSpecification));
         }
         else {
-            createSlobroks(admin, pickContainerHosts(nodesSpecification.count(), minSlobroksPerContainerCluster));
+            createSlobroks(admin, pickContainerHosts(nodesSpecification.count(), 2));
         }
     }
 
@@ -145,11 +144,4 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         admin.addSlobroks(slobroks);
     }
 
-    private int getMinSlobroksPerContainerCluster(ModelElement adminElement) {
-        ModelElement minNodes = adminElement.getChild("minSlobroksPerCluster");
-        if (minNodes == null) return 1; //default
-
-        return (int) minNodes.asLong();
-    }
-
 }
diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc
index c74a7cb6218..3db16f74e77 100644
--- a/config-model/src/main/resources/schema/admin.rnc
+++ b/config-model/src/main/resources/schema/admin.rnc
@@ -5,7 +5,7 @@ AdminV2 =
  element admin {
    attribute version { "2.0" } &
    element adminserver { service.attlist } &
-   element minSlobroksPerCluster { xsd:positiveInteger }? &
+   element minSlobroksPerCluster { xsd:positiveInteger }? & # TODO: Remove when no applications use this
    GenericConfig* &
    LogServer? &
    (ConfigServer | ConfigServers)? &
@@ -19,7 +19,7 @@ AdminV2 =
 AdminV3 =
   element admin {
     attribute version { "3.0"  } &
-    element minSlobroksPerCluster { xsd:positiveInteger }? &
+    element minSlobroksPerCluster { xsd:positiveInteger }? & # TODO: Remove when no applications use this
     GenericConfig* &
     Nodes
   }
@@ -27,7 +27,7 @@ AdminV3 =
 AdminV4 =
   element admin {
     attribute version { "4.0"  } &
-    element minSlobroksPerCluster { xsd:positiveInteger }? &
+    element minSlobroksPerCluster { xsd:positiveInteger }? & # TODO: Remove when no applications use this
     AdminV4Slobroks? &
     AdminV4LogServers? &
     GenericConfig* &
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
index c196318bde2..f6e7d50f643 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
@@ -82,7 +82,7 @@ public class DedicatedAdminV4Test {
     }
 
     @Test
-    public void testModelBuildingWithConfiguredMinSlobrokCountPerCluster() throws IOException, SAXException {
+    public void testThatThereAre2SlobroksPerContainerCluster() throws IOException, SAXException {
         String hosts = "<hosts>"
                 + "  <host name=\"myhost0\">"
                 + "    <alias>node0</alias>"
@@ -98,56 +98,36 @@ public class DedicatedAdminV4Test {
                 + "  </host>"
                 + "</hosts>";
 
-        {
-            VespaModel model = createModel(hosts, servicesWithMinSlobroksPerCluster(1));
-            assertEquals(4, model.getHosts().size());
-
-            // 3 slobroks, 1 per cluster
-            assertHostContainsServices(model, "hosts/myhost0",
-                                       "slobrok", "logd", "filedistributorservice", "logserver", "qrserver");
-            assertHostContainsServices(model, "hosts/myhost1",
-                                       "logd", "filedistributorservice", "qrserver");
-            assertHostContainsServices(model, "hosts/myhost2",
-                                       "slobrok", "logd", "filedistributorservice", "qrserver");
-            assertHostContainsServices(model, "hosts/myhost3",
-                                       "slobrok", "logd", "filedistributorservice", "qrserver");
-        }
-
-        {
-            VespaModel model = createModel(hosts, servicesWithMinSlobroksPerCluster(2));
-            assertEquals(4, model.getHosts().size());
-
-            // 4 slobroks, 2 per cluster where possible
-            assertHostContainsServices(model, "hosts/myhost0",
-                                       "slobrok", "logd", "filedistributorservice", "logserver", "qrserver");
-            assertHostContainsServices(model, "hosts/myhost1",
-                                       "slobrok", "logd", "filedistributorservice", "qrserver");
-            assertHostContainsServices(model, "hosts/myhost2",
-                                       "slobrok", "logd", "filedistributorservice", "qrserver");
-            assertHostContainsServices(model, "hosts/myhost3",
-                                       "slobrok", "logd", "filedistributorservice", "qrserver");
-        }
-    }
-
-    private String servicesWithMinSlobroksPerCluster(int count) {
-        return "<services>" +
-                "  <admin version='4.0'>" +
-                "    <minSlobroksPerCluster>" + count + "</minSlobroksPerCluster>" +
-                "    <nodes count='1' dedicated='true' />" +
-                "  </admin>" +
-                "  <jdisc id='a' version='1.0'>" +
-                "    <search />" +
-                "    <nodes count='2' dedicated='true' />" +
-                "  </jdisc>" +
-                "  <jdisc id='b' version='1.0'>" +
-                "    <search />" +
-                "    <nodes count='1' dedicated='true' />" +
-                "  </jdisc>" +
-                "  <jdisc id='c' version='1.0'>" +
-                "    <search />" +
-                "    <nodes count='1' dedicated='true' />" +
-                "  </jdisc>" +
-                "</services>";
+        String servicesWith3JdiscClusters = "<services>" +
+                    "  <admin version='4.0'>" +
+                    "    <nodes count='1' dedicated='true' />" +
+                    "  </admin>" +
+                    "  <jdisc id='a' version='1.0'>" +
+                    "    <search />" +
+                    "    <nodes count='2' dedicated='true' />" +
+                    "  </jdisc>" +
+                    "  <jdisc id='b' version='1.0'>" +
+                    "    <search />" +
+                    "    <nodes count='1' dedicated='true' />" +
+                    "  </jdisc>" +
+                    "  <jdisc id='c' version='1.0'>" +
+                    "    <search />" +
+                    "    <nodes count='1' dedicated='true' />" +
+                    "  </jdisc>" +
+                    "</services>";
+
+        VespaModel model = createModel(hosts, servicesWith3JdiscClusters);
+        assertEquals(4, model.getHosts().size());
+
+        // 4 slobroks, 2 per cluster where possible
+        assertHostContainsServices(model, "hosts/myhost0",
+                                   "slobrok", "logd", "filedistributorservice", "logserver", "qrserver");
+        assertHostContainsServices(model, "hosts/myhost1",
+                                   "slobrok", "logd", "filedistributorservice", "qrserver");
+        assertHostContainsServices(model, "hosts/myhost2",
+                                   "slobrok", "logd", "filedistributorservice", "qrserver");
+        assertHostContainsServices(model, "hosts/myhost3",
+                                   "slobrok", "logd", "filedistributorservice", "qrserver");
     }
 
     private Set<String> serviceNames(VespaModel model, String hostname) {