Merge pull request #26808 from vespa-engine/hmusum/handle-environments-where-number-of-groups-may-be-reduced

Hmusum/handle environments where number of groups may be reduced

6 files changed, 142 insertions, 60 deletions

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 5a96e33c522..e2166b263ee 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
@@ -9,6 +9,7 @@ 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.content.cluster.ContentCluster;
 import com.yahoo.vespa.model.utils.Duration;
 import org.w3c.dom.Element;
 import java.util.Optional;
@@ -47,9 +48,29 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc
                 clusterControllerTuning = tuning.child("cluster-controller");
             }
 
+            var tuningConfig = new ClusterControllerTuningBuilder(clusterControllerTuning,
+                                                            minNodeRatioPerGroup,
+                                                            bucketSplittingMinimumBits)
+                    .build();
+            if (ancestor instanceof ContentCluster) {
+                int numberOfLeafGroups = ((ContentCluster) ancestor).getRootGroup().getNumberOfLeafGroups();
+                if (tuningConfig.maxGroupsAllowedDown().isPresent()) {
+                    Integer maxGroupsAllowedDown = tuningConfig.maxGroupsAllowedDown().get();
+                    if (deployState.zone().environment().isProduction() && (maxGroupsAllowedDown > numberOfLeafGroups))
+                        throw new IllegalArgumentException("Cannot set max-groups-allowed-down (" + maxGroupsAllowedDown +
+                                                                   ") larger than number of groups (" + numberOfLeafGroups + ")");
+                } else {
+                    // Reduce to numberOfLeafGroups for tests or in environments where number of groups are reduced by policy (dev, test, staging, perf)
+                    tuningConfig = tuningConfig.withMaxGroupsAllowedDown(numberOfLeafGroups);
+                }
+            } else {
+                // Reduce to 1 for tests (ancestor is a mock class)
+                tuningConfig = tuningConfig.withMaxGroupsAllowedDown(1);
+            }
+
             return new ClusterControllerConfig(ancestor,
                                                clusterName,
-                                               new ClusterControllerTuning(clusterControllerTuning, minNodeRatioPerGroup, bucketSplittingMinimumBits),
+                                               tuningConfig,
                                                resourceLimits,
                                                allowMoreThanOneContentGroupDown);
         }
@@ -99,45 +120,81 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc
         resourceLimits.getConfig(builder);
     }
 
-    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 ClusterControllerTuning tuning() {return tuning;}
+
+private static class ClusterControllerTuningBuilder {
+
+    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;
+
+    ClusterControllerTuningBuilder(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; }
+    private ClusterControllerTuning build() {
+        return new ClusterControllerTuning(initProgressTime,
+                                           transitionTime,
+                                           maxPrematureCrashes,
+                                           stableStateTimePeriod,
+                                           minDistributorUpRatio,
+                                           minStorageUpRatio,
+                                           maxGroupsAllowedDown,
+                                           minNodeRatioPerGroup,
+                                           minSplitBits);
     }
 
 }
+
+private record ClusterControllerTuning(Optional<Duration> initProgressTime,
+                                       Optional<Duration> transitionTime,
+                                       Optional<Long> maxPrematureCrashes,
+                                       Optional<Duration> stableStateTimePeriod,
+                                       Optional<Double> minDistributorUpRatio,
+                                       Optional<Double> minStorageUpRatio,
+                                       Optional<Integer> maxGroupsAllowedDown,
+                                       Optional<Double> minNodeRatioPerGroup,
+                                       Optional<Integer> minSplitBits) {
+
+    public ClusterControllerTuning withMaxGroupsAllowedDown(int maxGroupsAllowedDown) {
+        return new ClusterControllerConfig.ClusterControllerTuning(
+                initProgressTime,
+                transitionTime,
+                maxPrematureCrashes,
+                stableStateTimePeriod,
+                minDistributorUpRatio,
+                minStorageUpRatio,
+                Optional.of(maxGroupsAllowedDown),
+                minNodeRatioPerGroup,
+                minSplitBits);
+    }
+
+}
+
+}
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 f1f210b013c..f1d5c7c9220 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
@@ -164,18 +164,12 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem
                                                                       DeployState deployState,
                                                                       ContentCluster c,
                                                                       ClusterResourceLimits resourceLimits) {
-            var config = new ClusterControllerConfig.Builder(c.clusterId,
-                                                             contentElement,
-                                                             resourceLimits.getClusterControllerLimits(),
-                                                             deployState.featureFlags()
-                                                                        .allowMoreThanOneContentGroupDown(new ClusterSpec.Id(c.clusterId)))
+            return 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) {
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 90b4625a282..f1dffe53ad7 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
@@ -15,6 +15,7 @@ import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.core.ApplicationMetadataConfig;
 import com.yahoo.search.config.QrStartConfig;
+import com.yahoo.vespa.config.content.FleetcontrollerConfig;
 import com.yahoo.vespa.config.content.core.StorCommunicationmanagerConfig;
 import com.yahoo.vespa.config.content.core.StorStatusConfig;
 import com.yahoo.vespa.config.search.core.ProtonConfig;
@@ -2352,6 +2353,38 @@ public class ModelProvisioningTest {
     }
 
     @Test
+    public void testAllow2ContentGroupsDown() {
+        String servicesXml =
+                "<?xml version='1.0' encoding='utf-8' ?>" +
+                        "<services>" +
+                        "  <container version='1.0' id='qrs'>" +
+                        "     <nodes count='1'/>" +
+                        "  </container>" +
+                        "  <content version='1.0' id='content'>" +
+                        "     <redundancy>1</redundancy>" +
+                        "     <documents>" +
+                        "       <document type='type1' mode='index'/>" +
+                        "     </documents>" +
+                        "    <nodes count='4' groups='4'/>" +
+                        "    <tuning>" +
+                        "      <cluster-controller>" +
+                        "        <max-groups-allowed-down>2</max-groups-allowed-down>" +
+                        "      </cluster-controller>" +
+                        "    </tuning>" +
+                        "  </content>" +
+                        "</services>";
+        VespaModelTester tester = new VespaModelTester();
+        tester.setModelProperties(new TestProperties().setAllowMoreThanOneContentGroupDown(true));
+        tester.addHosts(9);
+        VespaModel model = tester.createModel(servicesXml, true, new DeployState.Builder()
+                .properties(new TestProperties().setAllowMoreThanOneContentGroupDown(true)));
+
+        var fleetControllerConfigBuilder = new FleetcontrollerConfig.Builder();
+        model.getConfig(fleetControllerConfigBuilder, "admin/standalone/cluster-controllers/0/components/clustercontroller-content-configurer");
+        assertEquals(2, fleetControllerConfigBuilder.build().max_number_of_groups_allowed_to_be_down());
+    }
+
+    @Test
     public void containerWithZooKeeperSuboptimalNodeCountDuringRetirement() {
         String servicesXml =
                 "<?xml version='1.0' encoding='utf-8' ?>" +
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java
index a33b30f7d93..12a6ac00f48 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java
@@ -1331,14 +1331,10 @@ public class ContentClusterTest extends ContentBaseTest {
                 "    </engine>" +
                 "  </content>" +
                 " </services>";
-        VespaModel model = createEnd2EndOneNode(new TestProperties()
-                                                        .setHostedVespa(false)
-                                                        .setMultitenant(true)
-                                                        .setAllowMoreThanOneContentGroupDown(true),
-                                                services);
+        VespaModel model = createEnd2EndOneNode(new TestProperties().setAllowMoreThanOneContentGroupDown(true), services);
 
         var fleetControllerConfigBuilder = new FleetcontrollerConfig.Builder();
-        model.getConfig(fleetControllerConfigBuilder, "admin/standalone/cluster-controllers/0/components/clustercontroller-storage-configurer");
+        model.getConfig(fleetControllerConfigBuilder, "admin/cluster-controllers/0/components/clustercontroller-storage-configurer");
         assertEquals(2, fleetControllerConfigBuilder.build().max_number_of_groups_allowed_to_be_down());
     }
 
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 ae22542de6c..138852e1c5c 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
@@ -41,14 +41,14 @@ public class FleetControllerClusterTest {
         parse("""
                       <cluster id="storage">
                         <documents/>  <tuning>
-                          <bucket-splitting minimum-bits="7" />    <cluster-controller>
+                          <bucket-splitting minimum-bits="7" />
+                          <cluster-controller>
                             <init-progress-time>13</init-progress-time>
                             <transition-time>27</transition-time>
                             <max-premature-crashes>4</max-premature-crashes>
                             <stable-state-period>72</stable-state-period>
                             <min-distributor-up-ratio>0.7</min-distributor-up-ratio>
                             <min-storage-up-ratio>0.3</min-storage-up-ratio>
-                            <max-groups-allowed-down>2</max-groups-allowed-down>
                           </cluster-controller>
                         </tuning>
                       </cluster>""",
@@ -63,7 +63,6 @@ public class FleetControllerClusterTest {
         assertEquals(0.7, config.min_distributor_up_ratio(), 0.01);
         assertEquals(0.3, config.min_storage_up_ratio(), 0.01);
         assertEquals(7, config.ideal_distribution_bits());
-        assertEquals(2, config.max_number_of_groups_allowed_to_be_down());
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
index 48ddf6b8a82..500fb0838e1 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
@@ -21,7 +21,6 @@ import com.yahoo.config.provision.ProvisionLogger;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -50,6 +49,7 @@ public class VespaModelTester {
     private final ConfigModelRegistry configModelRegistry;
 
     private boolean hosted = true;
+    private TestProperties modelProperties = new TestProperties();
     private final Map<NodeResources, Collection<Host>> hostsByResources = new HashMap<>();
     private ApplicationId applicationId = ApplicationId.defaultId();
     private boolean useDedicatedNodeForLogserver = false;
@@ -101,6 +101,9 @@ public class VespaModelTester {
     /** Sets whether this sets up a model for a hosted system. Default: true */
     public void setHosted(boolean hosted) { this.hosted = hosted; }
 
+    /** Sets whether this sets up a model for a hosted system. Default: true */
+    public void setModelProperties(TestProperties testProperties) { this.modelProperties = testProperties; }
+
     /** Sets architecture to use for admin clusters. Default: x86_64 */
     public void setAdminClusterArchitecture(Architecture architecture) {
         this.adminClusterArchitecture = architecture;
@@ -206,7 +209,7 @@ public class VespaModelTester {
             provisioner = new SingleNodeProvisioner();
         }
 
-        TestProperties properties = new TestProperties()
+        TestProperties properties = modelProperties
                 .setMultitenant(hosted) // Note: system tests are multitenant but not hosted
                 .setHostedVespa(hosted)
                 .setApplicationId(applicationId)