Merge pull request #16340 from vespa-engine/geirst/resource-limits-in-cluster-tuning

Make it possible to specify feed block resource limits in content clu…

4 files changed, 210 insertions, 25 deletions

diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java
new file mode 100644
index 00000000000..bc830c079d0
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java
@@ -0,0 +1,101 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.model.content;
+
+import org.junit.Test;
+
+import java.util.Optional;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+/**
+ * @author geirst
+ */
+public class ClusterResourceLimitsTest {
+
+    private static class Fixture {
+        ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder();
+        ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder();
+
+        public Fixture ctrlDisk(double limit) {
+            ctrlBuilder.setDiskLimit(limit);
+            return this;
+        }
+        public Fixture ctrlMemory(double limit) {
+            ctrlBuilder.setMemoryLimit(limit);
+            return this;
+        }
+        public Fixture nodeDisk(double limit) {
+            nodeBuilder.setDiskLimit(limit);
+            return this;
+        }
+        public Fixture nodeMemory(double limit) {
+            nodeBuilder.setMemoryLimit(limit);
+            return this;
+        }
+        public ClusterResourceLimits build() {
+            var builder = new ClusterResourceLimits.Builder();
+            builder.setClusterControllerBuilder(ctrlBuilder);
+            builder.setContentNodeBuilder(nodeBuilder);
+            return builder.build();
+        }
+    }
+
+    @Test
+    public void content_node_limits_are_derived_from_cluster_controller_limits_if_not_set() {
+        assertLimits(0.6, 0.7, 0.8, 0.85,
+                new Fixture().ctrlDisk(0.6).ctrlMemory(0.7));
+        assertLimits(0.6, null, 0.8, null,
+                new Fixture().ctrlDisk(0.6));
+        assertLimits(null, 0.7, null, 0.85,
+                new Fixture().ctrlMemory(0.7));
+    }
+
+    @Test
+    public void content_node_limits_can_be_set_explicit() {
+        assertLimits(0.6, 0.7, 0.9, 0.95,
+                new Fixture().ctrlDisk(0.6).ctrlMemory(0.7).nodeDisk(0.9).nodeMemory(0.95));
+        assertLimits(0.6, null, 0.9, null,
+                new Fixture().ctrlDisk(0.6).nodeDisk(0.9));
+        assertLimits(null, 0.7, null, 0.95,
+                new Fixture().ctrlMemory(0.7).nodeMemory(0.95));
+    }
+
+    @Test
+    public void cluster_controller_limits_are_equal_to_content_node_limits_if_not_set() {
+        assertLimits(0.9, 0.95, 0.9, 0.95,
+                new Fixture().nodeDisk(0.9).nodeMemory(0.95));
+        assertLimits(0.9, null, 0.9, null,
+                new Fixture().nodeDisk(0.9));
+        assertLimits(null, 0.95, null, 0.95,
+                new Fixture().nodeMemory(0.95));
+    }
+
+    @Test
+    public void limits_are_derived_from_the_other_if_not_set() {
+        assertLimits(0.6, 0.95, 0.8, 0.95,
+                new Fixture().ctrlDisk(0.6).nodeMemory(0.95));
+        assertLimits(0.9, 0.7, 0.9, 0.85,
+                new Fixture().ctrlMemory(0.7).nodeDisk(0.9));
+    }
+
+    private void assertLimits(Double expCtrlDisk, Double expCtrlMemory, Double expNodeDisk, Double expNodeMemory, Fixture f) {
+        var limits = f.build();
+        assertLimits(expCtrlDisk, expCtrlMemory, limits.getClusterControllerLimits());
+        assertLimits(expNodeDisk, expNodeMemory, limits.getContentNodeLimits());
+    }
+
+    private void assertLimits(Double expDisk, Double expMemory, ResourceLimits limits) {
+        assertLimit(expDisk, limits.getDiskLimit());
+        assertLimit(expMemory, limits.getMemoryLimit());
+    }
+
+    private void assertLimit(Double expLimit, Optional<Double> actLimit) {
+        if (expLimit == null) {
+            assertFalse(actLimit.isPresent());
+        } else {
+            assertEquals(expLimit, actLimit.get(), 0.00001);
+        }
+    }
+
+}
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java
index 3415044b088..bc60908e268 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java
@@ -69,15 +69,30 @@ public class ContentSearchClusterTest {
     }
 
     private static ProtonConfig getProtonConfig(ContentCluster cluster) {
-        ProtonConfig.Builder protonCfgBuilder = new ProtonConfig.Builder();
-        cluster.getSearch().getConfig(protonCfgBuilder);
-        return new ProtonConfig(protonCfgBuilder);
+        var builder = new ProtonConfig.Builder();
+        cluster.getSearch().getConfig(builder);
+        return new ProtonConfig(builder);
     }
 
-    private static void assertProtonResourceLimits(double expDiskLimit, double expMemoryLimits, String clusterXml) throws Exception {
-        ProtonConfig cfg = getProtonConfig(createCluster(clusterXml));
+    private static void assertProtonResourceLimits(double expDiskLimit, double expMemoryLimit, String clusterXml) throws Exception {
+        assertProtonResourceLimits(expDiskLimit, expMemoryLimit, createCluster(clusterXml));
+    }
+
+    private static void assertProtonResourceLimits(double expDiskLimit, double expMemoryLimit, ContentCluster cluster) {
+        var cfg = getProtonConfig(cluster);
         assertEquals(expDiskLimit, cfg.writefilter().disklimit(), EPSILON);
-        assertEquals(expMemoryLimits, cfg.writefilter().memorylimit(), EPSILON);
+        assertEquals(expMemoryLimit, cfg.writefilter().memorylimit(), EPSILON);
+    }
+
+    private static void assertClusterControllerResourceLimits(double expDiskLimit, double expMemoryLimit, String clusterXml) throws Exception {
+        assertClusterControllerResourceLimits(expDiskLimit, expMemoryLimit, createCluster(clusterXml));
+    }
+
+    private static void assertClusterControllerResourceLimits(double expDiskLimit, double expMemoryLimit, ContentCluster cluster) {
+        var limits = getFleetcontrollerConfig(cluster).cluster_feed_block_limit();
+        assertEquals(4, limits.size());
+        assertEquals(expDiskLimit, limits.get("disk"), EPSILON);
+        assertEquals(expMemoryLimit, limits.get("memory"), EPSILON);
     }
 
     @Test
@@ -105,6 +120,19 @@ public class ContentSearchClusterTest {
     }
 
     @Test
+    public void cluster_controller_resource_limits_can_be_set() throws Exception {
+        assertClusterControllerResourceLimits(0.92, 0.93,
+                new ContentClusterBuilder().clusterControllerDiskLimit(0.92).clusterControllerMemoryLimit(0.93).getXml());
+    }
+
+    @Test
+    public void resource_limits_are_derived_from_the_other_if_not_specified() throws Exception {
+        var cluster = createCluster(new ContentClusterBuilder().clusterControllerDiskLimit(0.5).protonMemoryLimit(0.95).getXml());
+        assertProtonResourceLimits(0.75, 0.95, cluster);
+        assertClusterControllerResourceLimits(0.5, 0.95, cluster);
+    }
+
+    @Test
     public void requireThatGloballyDistributedDocumentTypeIsTaggedAsSuch() throws Exception {
         ProtonConfig cfg = getProtonConfig(createClusterWithGlobalType());
         assertEquals(2, cfg.documentdb().size());
@@ -149,8 +177,9 @@ public class ContentSearchClusterTest {
     }
 
     private static FleetcontrollerConfig getFleetcontrollerConfig(ContentCluster cluster) {
-        FleetcontrollerConfig.Builder builder = new FleetcontrollerConfig.Builder();
+        var builder = new FleetcontrollerConfig.Builder();
         cluster.getConfig(builder);
+        cluster.getClusterControllerConfig().getConfig(builder);
         builder.cluster_name("unknown");
         builder.index(0);
         builder.zookeeper_server("unknown");
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java
index 01bbffce360..3a59f35ce2e 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
 import org.junit.Test;
 import org.w3c.dom.Document;
 
+import static com.yahoo.config.model.test.TestUtil.joinLines;
 import static org.junit.Assert.assertEquals;
 
 public class FleetControllerClusterTest {
@@ -19,8 +20,11 @@ public class FleetControllerClusterTest {
         var deployState = new DeployState.Builder().properties(
                 new TestProperties().enableFeedBlockInDistributor(enableFeedBlockInDistributor)).build();
         MockRoot root = new MockRoot("", deployState);
-        return new ClusterControllerConfig.Builder("storage", new ModelElement(doc.getDocumentElement())).build(root.getDeployState(), root,
-                new ModelElement(doc.getDocumentElement()).getXml());
+        var clusterElement = new ModelElement(doc.getDocumentElement());
+        return new ClusterControllerConfig.Builder("storage",
+                clusterElement,
+                new ClusterResourceLimits.Builder().build(clusterElement).getClusterControllerLimits()).
+                build(root.getDeployState(), root, clusterElement.getXml());
     }
 
     private ClusterControllerConfig parse(String xml) {
@@ -94,15 +98,43 @@ public class FleetControllerClusterTest {
         assertEquals(0.0, config.min_node_ratio_per_group(), 0.01);
     }
 
+
     @Test
     public void default_cluster_feed_block_limits_are_set() {
-        var config = getConfigForBasicCluster();
+        assertLimits(0.79, 0.79, getConfigForBasicCluster());
+    }
+
+    @Test
+    public void resource_limits_can_be_set_in_tuning() {
+        assertLimits(0.6, 0.7, getConfigForResourceLimitsTuning(0.6, 0.7));
+        assertLimits(0.6, 0.79, getConfigForResourceLimitsTuning(0.6, null));
+        assertLimits(0.79, 0.7, getConfigForResourceLimitsTuning(null, 0.7));
+    }
+
+    private static double DELTA = 0.00001;
+
+    private void assertLimits(double expDisk, double expMemory, FleetcontrollerConfig config) {
         var limits = config.cluster_feed_block_limit();
         assertEquals(4, limits.size());
-        assertEquals(0.79, limits.get("memory"), 0.0001);
-        assertEquals(0.79, limits.get("disk"), 0.0001);
-        assertEquals(0.89, limits.get("attribute-enum-store"), 0.0001);
-        assertEquals(0.89, limits.get("attribute-multi-value"), 0.0001);
+        assertEquals(expDisk, limits.get("disk"), DELTA);
+        assertEquals(expMemory, limits.get("memory"), DELTA);
+        assertEquals(0.89, limits.get("attribute-enum-store"), DELTA);
+        assertEquals(0.89, limits.get("attribute-multi-value"), DELTA);
+    }
+
+    private FleetcontrollerConfig getConfigForResourceLimitsTuning(Double diskLimit, Double memoryLimit) {
+        FleetcontrollerConfig.Builder builder = new FleetcontrollerConfig.Builder();
+        parse(joinLines("<cluster id=\"test\">",
+                "<documents/>",
+                "<tuning>",
+                "  <resource-limits>",
+                (diskLimit != null ? ("    <disk>" + diskLimit + "</disk>") : ""),
+                (memoryLimit != null ? ("    <memory>" + memoryLimit + "</memory>") : ""),
+                "  </resource-limits>",
+                "</tuning>" +
+                "</cluster>")).
+                getConfig(builder);
+        return new FleetcontrollerConfig(builder);
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java
index 866c03d82f0..491326fdc9c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterBuilder.java
@@ -26,6 +26,8 @@ public class ContentClusterBuilder {
     private Optional<String> dispatchXml = Optional.empty();
     private Optional<Double> protonDiskLimit = Optional.empty();
     private Optional<Double> protonMemoryLimit = Optional.empty();
+    private Optional<Double> clusterControllerDiskLimit = Optional.empty();
+    private Optional<Double> clusterControllerMemoryLimit = Optional.empty();
 
     public ContentClusterBuilder() {
     }
@@ -67,13 +69,23 @@ public class ContentClusterBuilder {
         return this;
     }
 
-    public ContentClusterBuilder protonDiskLimit(double diskLimit) {
-        protonDiskLimit = Optional.of(diskLimit);
+    public ContentClusterBuilder protonDiskLimit(double limit) {
+        protonDiskLimit = Optional.of(limit);
         return this;
     }
 
-    public ContentClusterBuilder protonMemoryLimit(double memoryLimit) {
-        protonMemoryLimit = Optional.of(memoryLimit);
+    public ContentClusterBuilder protonMemoryLimit(double limit) {
+        protonMemoryLimit = Optional.of(limit);
+        return this;
+    }
+
+    public ContentClusterBuilder clusterControllerDiskLimit(double limit) {
+        clusterControllerDiskLimit = Optional.of(limit);
+        return this;
+    }
+
+    public ContentClusterBuilder clusterControllerMemoryLimit(double limit) {
+        clusterControllerMemoryLimit = Optional.of(limit);
         return this;
     }
 
@@ -88,14 +100,17 @@ public class ContentClusterBuilder {
                "  <engine>",
                "    <proton>",
                "      <searchable-copies>" + searchableCopies + "</searchable-copies>",
-               getResourceLimitsXml("      "),
+               getProtonResourceLimitsXml("      "),
                "    </proton>",
                "  </engine>");
         if (dispatchXml.isPresent()) {
             xml += dispatchXml.get();
         }
-        return xml + groupXml +
-               "</content>";
+        xml += groupXml;
+        xml += joinLines("  <tuning>",
+               getTuningResourceLimitsXml("    "),
+               "  </tuning>");
+        return xml + "</content>";
     }
 
     private static String getSimpleGroupXml() {
@@ -104,11 +119,19 @@ public class ContentClusterBuilder {
                 "  </group>");
     }
 
-    private String getResourceLimitsXml(String indent) {
-        if (protonDiskLimit.isPresent() || protonMemoryLimit.isPresent()) {
+    private String getProtonResourceLimitsXml(String indent) {
+        return getResourceLimitsXml(indent, protonDiskLimit, protonMemoryLimit);
+    }
+
+    private String getTuningResourceLimitsXml(String indent) {
+        return getResourceLimitsXml(indent, clusterControllerDiskLimit, clusterControllerMemoryLimit);
+    }
+
+    private String getResourceLimitsXml(String indent, Optional<Double> diskLimit, Optional<Double> memoryLimit) {
+        if (diskLimit.isPresent() || memoryLimit.isPresent()) {
             String xml = joinLines(indent + "<resource-limits>",
-                    getXmlLine("disk", protonDiskLimit, indent + "  "),
-                    getXmlLine("memory", protonMemoryLimit, indent + "  "),
+                    getXmlLine("disk", diskLimit, indent + "  "),
+                    getXmlLine("memory", memoryLimit, indent + "  "),
                     indent + "</resource-limits>");
             return xml;
         }