Merge pull request #16442 from vespa-engine/geirst/improve-deriving-of-resource-limits

Improve deriving of resource limits

7 files changed, 116 insertions, 31 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java
index 5324ee171ec..c7d56c9b4b5 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java
@@ -34,11 +34,15 @@ public class ClusterResourceLimits {
 
     public static class Builder {
 
+        private final boolean enableFeedBlockInDistributor;
         private ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder();
         private ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder();
 
-        public ClusterResourceLimits build(ModelElement clusterElem) {
+        public Builder(boolean enableFeedBlockInDistributor) {
+            this.enableFeedBlockInDistributor = enableFeedBlockInDistributor;
+        }
 
+        public ClusterResourceLimits build(ModelElement clusterElem) {
             ModelElement tuningElem = clusterElem.childByPath("tuning");
             if (tuningElem != null) {
                 ctrlBuilder = DomResourceLimitsBuilder.createBuilder(tuningElem);
@@ -67,6 +71,12 @@ public class ClusterResourceLimits {
         }
 
         private void deriveLimits() {
+            if (enableFeedBlockInDistributor) {
+                // This also ensures that content nodes limits are derived according to the formula in calcContentNodeLimit().
+                considerSettingDefaultClusterControllerLimit(ctrlBuilder.getDiskLimit(), nodeBuilder.getDiskLimit(), ctrlBuilder::setDiskLimit);
+                considerSettingDefaultClusterControllerLimit(ctrlBuilder.getMemoryLimit(), nodeBuilder.getMemoryLimit(), ctrlBuilder::setMemoryLimit);
+            }
+
             deriveClusterControllerLimit(ctrlBuilder.getDiskLimit(), nodeBuilder.getDiskLimit(), ctrlBuilder::setDiskLimit);
             deriveClusterControllerLimit(ctrlBuilder.getMemoryLimit(), nodeBuilder.getMemoryLimit(), ctrlBuilder::setMemoryLimit);
 
@@ -74,13 +84,22 @@ public class ClusterResourceLimits {
             deriveContentNodeLimit(nodeBuilder.getMemoryLimit(), ctrlBuilder.getMemoryLimit(), nodeBuilder::setMemoryLimit);
         }
 
+        private void considerSettingDefaultClusterControllerLimit(Optional<Double> clusterControllerLimit,
+                                                                  Optional<Double> contentNodeLimit,
+                                                                  Consumer<Double> setter) {
+            // TODO: remove this when feed block in distributor is default enabled.
+            if (!clusterControllerLimit.isPresent() && !contentNodeLimit.isPresent()) {
+                setter.accept(0.8);
+            }
+        }
+
         private void deriveClusterControllerLimit(Optional<Double> clusterControllerLimit,
                                                   Optional<Double> contentNodeLimit,
                                                   Consumer<Double> setter) {
             if (!clusterControllerLimit.isPresent()) {
                 contentNodeLimit.ifPresent(limit ->
-                        // TODO: emit warning when using cluster controller resource limits are default enabled.
-                        setter.accept(limit));
+                        // TODO: emit warning when feed block in distributor is default enabled.
+                        setter.accept(Double.max(0.0, (limit - 0.01))));
             }
         }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java
index e96ba47c6b3..a12e183b409 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ResourceLimits.java
@@ -35,8 +35,8 @@ public class ResourceLimits implements FleetcontrollerConfig.Producer, ProtonCon
         // Note: The resource categories must match the ones used in host info reporting
         // between content nodes and cluster controller:
         // storage/src/vespa/storage/persistence/filestorage/service_layer_host_info_reporter.cpp
-        builder.cluster_feed_block_limit.put("memory", memoryLimit.orElse(0.79));
-        builder.cluster_feed_block_limit.put("disk", diskLimit.orElse(0.79));
+        builder.cluster_feed_block_limit.put("memory", memoryLimit.orElse(0.8));
+        builder.cluster_feed_block_limit.put("disk", diskLimit.orElse(0.8));
         builder.cluster_feed_block_limit.put("attribute-enum-store", 0.89);
         builder.cluster_feed_block_limit.put("attribute-multi-value", 0.89);
     }
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 44de4a1abec..f63a81173d8 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
@@ -135,7 +135,8 @@ public class ContentCluster extends AbstractConfigProducer implements
             ContentCluster c = new ContentCluster(context.getParentProducer(), getClusterId(contentElement), documentDefinitions,
                                                   globallyDistributedDocuments, routingSelection,
                                                   deployState.zone(), deployState.isHosted());
-            var resourceLimits = new ClusterResourceLimits.Builder().build(contentElement);
+            boolean enableFeedBlockInDistributor = deployState.getProperties().featureFlags().enableFeedBlockInDistributor();
+            var resourceLimits = new ClusterResourceLimits.Builder(enableFeedBlockInDistributor).build(contentElement);
             c.clusterControllerConfig = new ClusterControllerConfig.Builder(getClusterId(contentElement),
                     contentElement,
                     resourceLimits.getClusterControllerLimits()).build(deployState, c, contentElement.getXml());
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
index bc830c079d0..da2d5e2e3a0 100644
--- 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
@@ -14,8 +14,17 @@ import static org.junit.Assert.assertFalse;
 public class ClusterResourceLimitsTest {
 
     private static class Fixture {
-        ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder();
-        ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder();
+        private final boolean enableFeedBlockInDistributor;
+        private final ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder();
+        private final ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder();
+
+        public Fixture() {
+            this.enableFeedBlockInDistributor = false;
+        }
+
+        public Fixture(boolean enableFeedBlockInDistributor) {
+            this.enableFeedBlockInDistributor = enableFeedBlockInDistributor;
+        }
 
         public Fixture ctrlDisk(double limit) {
             ctrlBuilder.setDiskLimit(limit);
@@ -34,7 +43,7 @@ public class ClusterResourceLimitsTest {
             return this;
         }
         public ClusterResourceLimits build() {
-            var builder = new ClusterResourceLimits.Builder();
+            var builder = new ClusterResourceLimits.Builder(enableFeedBlockInDistributor);
             builder.setClusterControllerBuilder(ctrlBuilder);
             builder.setContentNodeBuilder(nodeBuilder);
             return builder.build();
@@ -43,42 +52,68 @@ public class ClusterResourceLimitsTest {
 
     @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(0.4, 0.7, 0.7, 0.85,
+                new Fixture().ctrlDisk(0.4).ctrlMemory(0.7));
+        assertLimits(0.4, null, 0.7, null,
+                new Fixture().ctrlDisk(0.4));
         assertLimits(null, 0.7, null, 0.85,
                 new Fixture().ctrlMemory(0.7));
+
+
+        assertLimits(0.4, 0.7, 0.7, 0.85,
+                new Fixture(true).ctrlDisk(0.4).ctrlMemory(0.7));
+        assertLimits(0.4, 0.8, 0.7, 0.9,
+                new Fixture(true).ctrlDisk(0.4));
+        assertLimits(0.8, 0.7, 0.9, 0.85,
+                new Fixture(true).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(0.4, 0.7, 0.9, 0.95,
+                new Fixture().ctrlDisk(0.4).ctrlMemory(0.7).nodeDisk(0.9).nodeMemory(0.95));
+        assertLimits(0.4, null, 0.95, null,
+                new Fixture().ctrlDisk(0.4).nodeDisk(0.95));
         assertLimits(null, 0.7, null, 0.95,
                 new Fixture().ctrlMemory(0.7).nodeMemory(0.95));
+
+        assertLimits(0.4, 0.7, 0.9, 0.95,
+                new Fixture(true).ctrlDisk(0.4).ctrlMemory(0.7).nodeDisk(0.9).nodeMemory(0.95));
+        assertLimits(0.4, 0.8, 0.95, 0.9,
+                new Fixture(true).ctrlDisk(0.4).nodeDisk(0.95));
+        assertLimits(0.8, 0.7, 0.9, 0.95,
+                new Fixture(true).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,
+    public void cluster_controller_limits_are_equal_to_content_node_limits_minus_one_percent_if_not_set() {
+        assertLimits(0.89, 0.94, 0.9, 0.95,
                 new Fixture().nodeDisk(0.9).nodeMemory(0.95));
-        assertLimits(0.9, null, 0.9, null,
+        assertLimits(0.89, null, 0.9, null,
                 new Fixture().nodeDisk(0.9));
-        assertLimits(null, 0.95, null, 0.95,
+        assertLimits(null, 0.94, null, 0.95,
                 new Fixture().nodeMemory(0.95));
+        assertLimits(null, 0.0, null, 0.005,
+                new Fixture().nodeMemory(0.005));
+
+        assertLimits(0.89, 0.94, 0.9, 0.95,
+                new Fixture(true).nodeDisk(0.9).nodeMemory(0.95));
     }
 
     @Test
     public void limits_are_derived_from_the_other_if_not_set() {
-        assertLimits(0.6, 0.95, 0.8, 0.95,
+        assertLimits(0.6, 0.94, 0.8, 0.95,
                 new Fixture().ctrlDisk(0.6).nodeMemory(0.95));
-        assertLimits(0.9, 0.7, 0.9, 0.85,
+        assertLimits(0.89, 0.7, 0.9, 0.85,
                 new Fixture().ctrlMemory(0.7).nodeDisk(0.9));
     }
 
+    @Test
+    public void default_resource_limits_when_feed_block_is_enabled_in_distributor() {
+        assertLimits(0.8, 0.8, 0.9, 0.9,
+                new Fixture(true));
+    }
+
     private void assertLimits(Double expCtrlDisk, Double expCtrlMemory, Double expNodeDisk, Double expNodeMemory, Fixture f) {
         var limits = f.build();
         assertLimits(expCtrlDisk, expCtrlMemory, limits.getClusterControllerLimits());
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 bc60908e268..50495a211e2 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
@@ -1,6 +1,8 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.content;
 
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.config.model.deploy.TestProperties;
 import com.yahoo.vespa.config.content.FleetcontrollerConfig;
 import com.yahoo.vespa.config.content.AllClustersBucketSpacesConfig;
 import com.yahoo.vespa.config.content.core.BucketspacesConfig;
@@ -74,6 +76,12 @@ public class ContentSearchClusterTest {
         return new ProtonConfig(builder);
     }
 
+    private static ContentCluster createClusterWithFeatureFlag(String clusterXml, boolean enableFeedBlockInDistributor) throws Exception {
+        var deployStateBuilder = new DeployState.Builder().properties(
+                new TestProperties().enableFeedBlockInDistributor(enableFeedBlockInDistributor));
+        return createCluster(clusterXml, deployStateBuilder);
+    }
+
     private static void assertProtonResourceLimits(double expDiskLimit, double expMemoryLimit, String clusterXml) throws Exception {
         assertProtonResourceLimits(expDiskLimit, expMemoryLimit, createCluster(clusterXml));
     }
@@ -129,7 +137,21 @@ public class ContentSearchClusterTest {
     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);
+        assertClusterControllerResourceLimits(0.5, 0.94, cluster);
+    }
+
+    @Test
+    public void default_resource_limits_when_feed_block_is_disabled_in_distributor() throws Exception {
+        var cluster = createClusterWithFeatureFlag(new ContentClusterBuilder().getXml(), false);
+        assertProtonResourceLimits(0.8, 0.8, cluster);
+        assertClusterControllerResourceLimits(0.8, 0.8, cluster);
+    }
+
+    @Test
+    public void default_resource_limits_when_feed_block_is_enabled_in_distributor() throws Exception {
+        var cluster = createClusterWithFeatureFlag(new ContentClusterBuilder().getXml(), true);
+        assertProtonResourceLimits(0.9, 0.9, cluster);
+        assertClusterControllerResourceLimits(0.8, 0.8, cluster);
     }
 
     @Test
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 3a59f35ce2e..32b84d95cc8 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
@@ -23,7 +23,7 @@ public class FleetControllerClusterTest {
         var clusterElement = new ModelElement(doc.getDocumentElement());
         return new ClusterControllerConfig.Builder("storage",
                 clusterElement,
-                new ClusterResourceLimits.Builder().build(clusterElement).getClusterControllerLimits()).
+                new ClusterResourceLimits.Builder(enableFeedBlockInDistributor).build(clusterElement).getClusterControllerLimits()).
                 build(root.getDeployState(), root, clusterElement.getXml());
     }
 
@@ -101,14 +101,14 @@ public class FleetControllerClusterTest {
 
     @Test
     public void default_cluster_feed_block_limits_are_set() {
-        assertLimits(0.79, 0.79, getConfigForBasicCluster());
+        assertLimits(0.8, 0.8, 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));
+        assertLimits(0.6, 0.8, getConfigForResourceLimitsTuning(0.6, null));
+        assertLimits(0.8, 0.7, getConfigForResourceLimitsTuning(null, 0.7));
     }
 
     private static double DELTA = 0.00001;
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java
index 297c0cebbb4..b816d803276 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java
@@ -68,16 +68,24 @@ public class ContentClusterUtils {
         return new ContentCluster.Builder(admin).build(Collections.emptyList(), context, doc.getDocumentElement());
     }
 
-    public static ContentCluster createCluster(String clusterXml, List<String> schemas) throws Exception {
-        MockRoot root = createMockRoot(schemas);
+    public static ContentCluster createCluster(String clusterXml, List<String> schemas, DeployState.Builder deployStateBuilder) throws Exception {
+        MockRoot root = createMockRoot(schemas, deployStateBuilder);
         ContentCluster cluster = createCluster(clusterXml, root);
         root.freezeModelTopology();
         cluster.validate();
         return cluster;
     }
 
+    public static ContentCluster createCluster(String clusterXml, List<String> schemas) throws Exception {
+        return createCluster(clusterXml, schemas, new DeployState.Builder());
+    }
+
     public static ContentCluster createCluster(String clusterXml) throws Exception {
-        return createCluster(clusterXml, SchemaBuilder.createSchemas("test"));
+        return createCluster(clusterXml, SchemaBuilder.createSchemas("test"), new DeployState.Builder());
+    }
+
+    public static ContentCluster createCluster(String clusterXml, DeployState.Builder deployStateBuilder) throws Exception {
+        return createCluster(clusterXml, SchemaBuilder.createSchemas("test"), deployStateBuilder);
     }
 
     public static String createClusterXml(String groupXml, int redundancy, int searchableCopies) {