Merge pull request #26796 from vespa-engine/hmusum/add-config-for-max-groups-allowed-down

Hmusum/add config for max groups allowed down

4 files changed, 101 insertions, 91 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index c72aa23a836..540905bef4c 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -83,6 +83,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private boolean useRestrictedDataPlaneBindings = false;
     private Optional<CloudAccount> cloudAccount = Optional.empty();
     private boolean allowUserFilters = true;
+    private boolean allowMoreThanOneContentGroupDown = false;
 
     @Override public ModelContext.FeatureFlags featureFlags() { return this; }
     @Override public boolean multitenant() { return multitenant; }
@@ -140,6 +141,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public Optional<CloudAccount> cloudAccount() { return cloudAccount; }
     @Override public boolean allowUserFilters() { return allowUserFilters; }
     @Override public boolean enableGlobalPhase() { return true; } // Enable global-phase by default for unit tests only
+    @Override public boolean allowMoreThanOneContentGroupDown(ClusterSpec.Id id) { return allowMoreThanOneContentGroupDown; }
 
     public TestProperties sharedStringRepoNoReclaim(boolean sharedStringRepoNoReclaim) {
         this.sharedStringRepoNoReclaim = sharedStringRepoNoReclaim;
@@ -368,6 +370,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         return this;
     }
 
+    public TestProperties setAllowMoreThanOneContentGroupDown(boolean allowMoreThanOneContentGroupDown) {
+        this.allowMoreThanOneContentGroupDown = allowMoreThanOneContentGroupDown;
+        return this;
+    }
+
     public TestProperties setAllowUserFilters(boolean b) { this.allowUserFilters = b; return this; }
 
     public static class Spec implements ConfigServerSpec {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerConfigurer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerConfigurer.java
index d63301a9668..25cca42f703 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerConfigurer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerConfigurer.java
@@ -16,16 +16,16 @@ public class ClusterControllerConfigurer extends SimpleComponent implements Stor
         FleetcontrollerConfig.Producer
 {
     private final ContentCluster cluster;
-    private final int index;
+    private final int clusterControllerIndex;
     private final int nodeCount;
 
-    public ClusterControllerConfigurer(ContentCluster cluster, int index, int nodeCount) {
+    public ClusterControllerConfigurer(ContentCluster cluster, int clusterControllerIndex, int nodeCount) {
         super(new ComponentModel(new BundleInstantiationSpecification(
                 new ComponentSpecification("clustercontroller" + "-" + cluster.getName() + "-configurer"),
                 new ComponentSpecification("com.yahoo.vespa.clustercontroller.apps.clustercontroller.ClusterControllerClusterConfigurer"),
                 new ComponentSpecification("clustercontroller-apps"))));
         this.cluster = cluster;
-        this.index = index;
+        this.clusterControllerIndex = clusterControllerIndex;
         this.nodeCount = nodeCount;
     }
 
@@ -38,7 +38,7 @@ public class ClusterControllerConfigurer extends SimpleComponent implements Stor
     public void getConfig(FleetcontrollerConfig.Builder builder) {
         cluster.getConfig(builder);
         cluster.getClusterControllerConfig().getConfig(builder);
-        builder.index(index);
+        builder.index(clusterControllerIndex);
         builder.fleet_controller_count(nodeCount);
         builder.http_port(0);
         builder.rpc_port(0);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java
index 201e0b5693a..5a96e33c522 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java
@@ -2,15 +2,16 @@
 package com.yahoo.vespa.model.content;
 
 import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
 import com.yahoo.config.model.producer.AnyConfigProducer;
 import com.yahoo.config.model.producer.TreeConfigProducer;
-import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
 import com.yahoo.vespa.config.content.FleetcontrollerConfig;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
 import com.yahoo.vespa.model.builder.xml.dom.VespaDomBuilder;
 import com.yahoo.vespa.model.utils.Duration;
 import org.w3c.dom.Element;
+import java.util.Optional;
 
 /**
  * Config generation for common parameters for all fleet controllers.
@@ -35,76 +36,38 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc
 
         @Override
         protected ClusterControllerConfig doBuild(DeployState deployState, TreeConfigProducer<AnyConfigProducer> ancestor, Element producerSpec) {
-            ModelElement tuning = null;
-
-            ModelElement clusterTuning = clusterElement.child("tuning");
-            Integer bucketSplittingMinimumBits = null;
-            Double minNodeRatioPerGroup = deployState.getProperties().featureFlags().minNodeRatioPerGroup();
-            if (clusterTuning != null) {
-                tuning = clusterTuning.child("cluster-controller");
-                minNodeRatioPerGroup = clusterTuning.childAsDouble("min-node-ratio-per-group");
-                bucketSplittingMinimumBits = clusterTuning.childAsInteger("bucket-splitting.minimum-bits");
-            }
+            ModelElement tuning = clusterElement.child("tuning");
+            ModelElement clusterControllerTuning = null;
 
+            Optional<Double> minNodeRatioPerGroup = Optional.of(deployState.featureFlags().minNodeRatioPerGroup());
+            Optional<Integer> bucketSplittingMinimumBits = Optional.empty();
             if (tuning != null) {
-                return new ClusterControllerConfig(ancestor, clusterName,
-                        tuning.childAsDuration("init-progress-time"),
-                        tuning.childAsDuration("transition-time"),
-                        tuning.childAsLong("max-premature-crashes"),
-                        tuning.childAsDuration("stable-state-period"),
-                        tuning.childAsDouble("min-distributor-up-ratio"),
-                        tuning.childAsDouble("min-storage-up-ratio"),
-                        bucketSplittingMinimumBits,
-                        minNodeRatioPerGroup,
-                        resourceLimits,
-                        allowMoreThanOneContentGroupDown);
-            } else {
-                return new ClusterControllerConfig(ancestor, clusterName,
-                        null, null, null, null, null, null,
-                        bucketSplittingMinimumBits,
-                        minNodeRatioPerGroup,
-                        resourceLimits,
-                        allowMoreThanOneContentGroupDown);
+                minNodeRatioPerGroup = Optional.ofNullable(tuning.childAsDouble("min-node-ratio-per-group"));
+                bucketSplittingMinimumBits = Optional.ofNullable(tuning.childAsInteger("bucket-splitting.minimum-bits"));
+                clusterControllerTuning = tuning.child("cluster-controller");
             }
+
+            return new ClusterControllerConfig(ancestor,
+                                               clusterName,
+                                               new ClusterControllerTuning(clusterControllerTuning, minNodeRatioPerGroup, bucketSplittingMinimumBits),
+                                               resourceLimits,
+                                               allowMoreThanOneContentGroupDown);
         }
     }
 
     private final String clusterName;
-    private final Duration initProgressTime;
-    private final Duration transitionTime;
-    private final Long maxPrematureCrashes;
-    private final Duration stableStateTimePeriod;
-    private final Double minDistributorUpRatio;
-    private final Double minStorageUpRatio;
-    private final Integer minSplitBits;
-    private final Double minNodeRatioPerGroup;
+    private final ClusterControllerTuning tuning;
     private final ResourceLimits resourceLimits;
     private final boolean allowMoreThanOneContentGroupDown;
 
-    // TODO refactor; too many args
     private ClusterControllerConfig(TreeConfigProducer<?> parent,
                                     String clusterName,
-                                    Duration initProgressTime,
-                                    Duration transitionTime,
-                                    Long maxPrematureCrashes,
-                                    Duration stableStateTimePeriod,
-                                    Double minDistributorUpRatio,
-                                    Double minStorageUpRatio,
-                                    Integer minSplitBits,
-                                    Double minNodeRatioPerGroup,
+                                    ClusterControllerTuning tuning,
                                     ResourceLimits resourceLimits,
                                     boolean allowMoreThanOneContentGroupDown) {
         super(parent, "fleetcontroller");
-
         this.clusterName = clusterName;
-        this.initProgressTime = initProgressTime;
-        this.transitionTime = transitionTime;
-        this.maxPrematureCrashes = maxPrematureCrashes;
-        this.stableStateTimePeriod = stableStateTimePeriod;
-        this.minDistributorUpRatio = minDistributorUpRatio;
-        this.minStorageUpRatio = minStorageUpRatio;
-        this.minSplitBits = minSplitBits;
-        this.minNodeRatioPerGroup = minNodeRatioPerGroup;
+        this.tuning = tuning;
         this.resourceLimits = resourceLimits;
         this.allowMoreThanOneContentGroupDown = allowMoreThanOneContentGroupDown;
     }
@@ -113,8 +76,7 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc
     public void getConfig(FleetcontrollerConfig.Builder builder) {
         AbstractConfigProducerRoot root = getRoot();
         if (root instanceof VespaModel) {
-            String zooKeeperAddress =
-                    root.getAdmin().getZooKeepersConfigProvider().getZooKeepersConnectionSpec();
+            String zooKeeperAddress = root.getAdmin().getZooKeepersConfigProvider().getZooKeepersConnectionSpec();
             builder.zookeeper_server(zooKeeperAddress);
         } else {
             builder.zookeeper_server("");
@@ -124,32 +86,58 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc
         builder.cluster_name(clusterName);
         builder.fleet_controller_count(getChildren().size());
 
-        if (initProgressTime != null) {
-            builder.init_progress_time((int) initProgressTime.getMilliSeconds());
-        }
-        if (transitionTime != null) {
-            builder.storage_transition_time((int) transitionTime.getMilliSeconds());
-        }
-        if (maxPrematureCrashes != null) {
-            builder.max_premature_crashes(maxPrematureCrashes.intValue());
-        }
-        if (stableStateTimePeriod != null) {
-            builder.stable_state_time_period((int) stableStateTimePeriod.getMilliSeconds());
-        }
-        if (minDistributorUpRatio != null) {
-            builder.min_distributor_up_ratio(minDistributorUpRatio);
-        }
-        if (minStorageUpRatio != null) {
-            builder.min_storage_up_ratio(minStorageUpRatio);
-        }
-        if (minSplitBits != null) {
-            builder.ideal_distribution_bits(minSplitBits);
-        }
-        if (minNodeRatioPerGroup != null) {
-            builder.min_node_ratio_per_group(minNodeRatioPerGroup);
-        }
+        tuning.initProgressTime.ifPresent(i -> builder.init_progress_time((int) i.getMilliSeconds()));
+        tuning.transitionTime.ifPresent(t -> builder.storage_transition_time((int) t.getMilliSeconds()));
+        tuning.maxPrematureCrashes.ifPresent(var -> builder.max_premature_crashes(var.intValue()));
+        tuning.stableStateTimePeriod.ifPresent(var -> builder.stable_state_time_period((int) var.getMilliSeconds()));
+        tuning.minDistributorUpRatio.ifPresent(builder::min_distributor_up_ratio);
+        tuning.minStorageUpRatio.ifPresent(builder::min_storage_up_ratio);
+        tuning.minSplitBits.ifPresent(builder::ideal_distribution_bits);
+        tuning.minNodeRatioPerGroup.ifPresent(builder::min_node_ratio_per_group);
+        tuning.maxGroupsAllowedDown.ifPresent(max -> builder.max_number_of_groups_allowed_to_be_down(allowMoreThanOneContentGroupDown ? max : -1));
+
         resourceLimits.getConfig(builder);
-        builder.max_number_of_groups_allowed_to_be_down(allowMoreThanOneContentGroupDown ? 1 : -1);
+    }
+
+    public ClusterControllerTuning tuning() { return tuning; }
+
+    public static class ClusterControllerTuning {
+
+        private final Optional<Double> minNodeRatioPerGroup;
+        private final Optional<Duration> initProgressTime;
+        private final Optional<Duration> transitionTime;
+        private final Optional<Long> maxPrematureCrashes;
+        private final Optional<Duration> stableStateTimePeriod;
+        private final Optional<Double> minDistributorUpRatio;
+        private final Optional<Double> minStorageUpRatio;
+        private final Optional<Integer> minSplitBits;
+        final Optional<Integer> maxGroupsAllowedDown;
+
+        ClusterControllerTuning(ModelElement tuning,
+                                Optional<Double> minNodeRatioPerGroup,
+                                Optional<Integer> bucketSplittingMinimumBits) {
+            this.minSplitBits = bucketSplittingMinimumBits;
+            this.minNodeRatioPerGroup = minNodeRatioPerGroup;
+            if (tuning == null) {
+                this.initProgressTime = Optional.empty();
+                this.transitionTime = Optional.empty();
+                this.maxPrematureCrashes = Optional.empty();
+                this.stableStateTimePeriod = Optional.empty();
+                this.minDistributorUpRatio = Optional.empty();
+                this.minStorageUpRatio = Optional.empty();
+                this.maxGroupsAllowedDown = Optional.empty();
+            } else {
+                this.initProgressTime = Optional.ofNullable(tuning.childAsDuration("init-progress-time"));
+                this.transitionTime = Optional.ofNullable(tuning.childAsDuration("transition-time"));
+                this.maxPrematureCrashes = Optional.ofNullable(tuning.childAsLong("max-premature-crashes"));
+                this.stableStateTimePeriod = Optional.ofNullable(tuning.childAsDuration("stable-state-period"));
+                this.minDistributorUpRatio = Optional.ofNullable(tuning.childAsDouble("min-distributor-up-ratio"));
+                this.minStorageUpRatio = Optional.ofNullable(tuning.childAsDouble("min-storage-up-ratio"));
+                this.maxGroupsAllowedDown = Optional.ofNullable(tuning.childAsInteger("max-groups-allowed-down"));
+            }
+        }
+
+        public Optional<Integer> maxGroupsAllowedDown() { return maxGroupsAllowedDown; }
     }
 
 }
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 217c26516a9..f1f210b013c 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
@@ -125,11 +125,6 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem
                                                                    deployState.featureFlags().resourceLimitDisk(),
                                                                    deployState.featureFlags().resourceLimitMemory())
                     .build(contentElement);
-            c.clusterControllerConfig = new ClusterControllerConfig.Builder(clusterId,
-                                                                            contentElement,
-                                                                            resourceLimits.getClusterControllerLimits(),
-                                                                            deployState.featureFlags().allowMoreThanOneContentGroupDown(new ClusterSpec.Id(clusterId)))
-                    .build(deployState, c, contentElement.getXml());
             c.search = new ContentSearchCluster.Builder(documentDefinitions,
                                                         globallyDistributedDocuments,
                                                         fractionOfMemoryReserved(clusterId, containers),
@@ -139,6 +134,7 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem
             c.storageNodes = new StorageCluster.Builder().build(deployState, c, w3cContentElement);
             c.distributorNodes = new DistributorCluster.Builder(c).build(deployState, c, w3cContentElement);
             c.rootGroup = new StorageGroup.Builder(contentElement, context).buildRootGroup(deployState, redundancyBuilder, c);
+            c.clusterControllerConfig = createClusterControllerConfig(contentElement, deployState, c, resourceLimits);
             validateThatGroupSiblingsAreUnique(c.clusterId, c.rootGroup);
             c.search.handleRedundancy(c.redundancy);
             setupSearchCluster(c.search, contentElement, deployState.getDeployLogger());
@@ -164,6 +160,24 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem
             return c;
         }
 
+        private ClusterControllerConfig createClusterControllerConfig(ModelElement contentElement,
+                                                                      DeployState deployState,
+                                                                      ContentCluster c,
+                                                                      ClusterResourceLimits resourceLimits) {
+            var config = new ClusterControllerConfig.Builder(c.clusterId,
+                                                             contentElement,
+                                                             resourceLimits.getClusterControllerLimits(),
+                                                             deployState.featureFlags()
+                                                                        .allowMoreThanOneContentGroupDown(new ClusterSpec.Id(c.clusterId)))
+                    .build(deployState, c, contentElement.getXml());
+            config.tuning().maxGroupsAllowedDown().ifPresent(m -> {
+                int numberOfLeafGroups = c.getRootGroup().getNumberOfLeafGroups();
+                if (m > numberOfLeafGroups)
+                    throw new IllegalArgumentException("Cannot set max-groups-allowed-down (" + m + ") larger than number of groups (" + numberOfLeafGroups + ")");
+            });
+            return config;
+        }
+
         private void setupSearchCluster(ContentSearchCluster csc, ModelElement element, DeployLogger logger) {
             ContentSearch search = DomContentSearchBuilder.build(element);
             Double visibilityDelay = search.getVisibilityDelay();
@@ -435,6 +449,7 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem
     public final ContentSearchCluster getSearch() { return search; }
 
     public Redundancy redundancy() { return redundancy; }
+
     public ContentCluster setRedundancy(Redundancy redundancy) {
         this.redundancy = redundancy;
         return this;