Refactor cluster controller tuning config

2 files changed, 66 insertions, 85 deletions

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..49d63cbfba3 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,51 @@ 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);
+
         resourceLimits.getConfig(builder);
         builder.max_number_of_groups_allowed_to_be_down(allowMoreThanOneContentGroupDown ? 1 : -1);
     }
 
+    private 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;
+
+        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();
+            } 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"));
+            }
+        }
+    }
+
 }