Merge pull request #18958 from vespa-engine/geirst/tune-num-distributor-stripes-based-on-flavor

Tune num distributor stripes based on flavor when the feature flag is…

4 files changed, 84 insertions, 28 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/Distributor.java b/config-model/src/main/java/com/yahoo/vespa/model/content/Distributor.java
index ec677911da6..f0a2e52f6bf 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/Distributor.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/Distributor.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.model.content;
 import com.yahoo.config.model.api.ModelContext;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.vespa.config.content.core.StorCommunicationmanagerConfig;
+import com.yahoo.vespa.config.content.core.StorDistributormanagerConfig;
 import com.yahoo.vespa.config.content.core.StorServerConfig;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
@@ -14,9 +15,10 @@ import org.w3c.dom.Element;
 /**
  * Represents specific configuration for a given distributor node.
  */
-public class Distributor extends ContentNode {
+public class Distributor extends ContentNode implements StorDistributormanagerConfig.Producer {
 
     PersistenceEngine provider;
+    private final int numDistributorStripesFlag;
 
     public static class Builder extends VespaDomBuilder.DomConfigProducerBuilder<Distributor> {
         ModelElement clusterXml;
@@ -39,12 +41,27 @@ public class Distributor extends ContentNode {
              StorageNode.rootFolder + parent.getClusterName() + "/distributor/" + distributionKey, distributionKey);
 
         this.provider = provider;
+        this.numDistributorStripesFlag = properties.featureFlags().numDistributorStripes();
 
         if (distributorBasePort != null) {
             setBasePort(distributorBasePort);
         }
     }
 
+    private int tuneNumDistributorStripes() {
+        if ((numDistributorStripesFlag == -1) && (getHostResource() != null)) {
+            int cores = (int)getHostResource().realResources().vcpu();
+            if (cores <= 16) {
+                return 1;
+            } else if (cores <= 64) {
+                return 2;
+            } else {
+                return 4;
+            }
+        }
+        return numDistributorStripesFlag;
+    }
+
     @Override
     public void getConfig(StorServerConfig.Builder builder) {
         super.getConfig(builder);
@@ -59,6 +76,11 @@ public class Distributor extends ContentNode {
     }
 
     @Override
+    public void getConfig(StorDistributormanagerConfig.Builder builder) {
+        builder.num_distributor_stripes(tuneNumDistributorStripes());
+    }
+
+    @Override
     public String getStartupCommand() {
         return "exec sbin/vespa-distributord -c $VESPA_CONFIG_ID";
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java
index 390c939c4b4..0e3fee5a749 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/DistributorCluster.java
@@ -43,7 +43,6 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl
     private final boolean hasIndexedDocumentType;
     private final boolean useThreePhaseUpdates;
     private final int maxActivationInhibitedOutOfSyncGroups;
-    private final int numDistributorStripes;
 
     public static class Builder extends VespaDomBuilder.DomConfigProducerBuilder<DistributorCluster> {
 
@@ -106,20 +105,18 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl
             final boolean hasIndexedDocumentType = clusterContainsIndexedDocumentType(documentsNode);
             boolean useThreePhaseUpdates = deployState.getProperties().featureFlags().useThreePhaseUpdates();
             int maxInhibitedGroups = deployState.getProperties().featureFlags().maxActivationInhibitedOutOfSyncGroups();
-            int numDistributorStripes = deployState.getProperties().featureFlags().numDistributorStripes();
 
             return new DistributorCluster(parent,
                     new BucketSplitting.Builder().build(new ModelElement(producerSpec)), gc,
                     hasIndexedDocumentType, useThreePhaseUpdates,
-                    maxInhibitedGroups, numDistributorStripes);
+                    maxInhibitedGroups);
         }
     }
 
     private DistributorCluster(ContentCluster parent, BucketSplitting bucketSplitting,
                                GcOptions gc, boolean hasIndexedDocumentType,
                                boolean useThreePhaseUpdates,
-                               int maxActivationInhibitedOutOfSyncGroups,
-                               int numDistributorStripes)
+                               int maxActivationInhibitedOutOfSyncGroups)
     {
         super(parent, "distributor");
         this.parent = parent;
@@ -128,7 +125,6 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl
         this.hasIndexedDocumentType = hasIndexedDocumentType;
         this.useThreePhaseUpdates = useThreePhaseUpdates;
         this.maxActivationInhibitedOutOfSyncGroups = maxActivationInhibitedOutOfSyncGroups;
-        this.numDistributorStripes = numDistributorStripes;
     }
 
     @Override
@@ -142,7 +138,6 @@ public class DistributorCluster extends AbstractConfigProducer<Distributor> impl
         builder.disable_bucket_activation(hasIndexedDocumentType == false);
         builder.enable_metadata_only_fetch_phase_for_inconsistent_updates(useThreePhaseUpdates);
         builder.max_activation_inhibited_out_of_sync_groups(maxActivationInhibitedOutOfSyncGroups);
-        builder.num_distributor_stripes(numDistributorStripes);
 
         bucketSplitting.getConfig(builder);
     }
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 13d02fc1fb8..c79238ed1d6 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
@@ -4,12 +4,15 @@ package com.yahoo.vespa.model.content;
 import com.yahoo.config.model.api.ModelContext;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
+import com.yahoo.config.model.provision.SingleNodeProvisioner;
 import com.yahoo.config.model.test.MockRoot;
 import com.yahoo.config.model.test.TestDriver;
 import com.yahoo.config.model.test.TestRoot;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
+import com.yahoo.config.provisioning.FlavorsConfig;
 import com.yahoo.container.ComponentsConfig;
 import com.yahoo.messagebus.routing.RoutingTableSpec;
 import com.yahoo.metrics.MetricsmanagerConfig;
@@ -798,13 +801,25 @@ public class ContentClusterTest extends ContentBaseTest {
         assertPrepareRestartCommand(createClusterWithFlushOnShutdownOverride(true, true));
     }
 
-    private static ContentCluster createOneNodeCluster(boolean isHostedVespa) throws Exception {
-        return createOneNodeCluster("<content version=\"1.0\" id=\"mockcluster\">" +
+    private static String oneNodeClusterXml() {
+        return "<content version=\"1.0\" id=\"mockcluster\">" +
                 "  <documents/>" +
                 "  <group>" +
                 "    <node distribution-key=\"0\" hostalias=\"mockhost\"/>" +
                 "  </group>" +
-                "</content>", isHostedVespa);
+                "</content>";
+    }
+
+    private static ContentCluster createOneNodeCluster(boolean isHostedVespa) throws Exception {
+        return createOneNodeCluster(oneNodeClusterXml(), new TestProperties().setHostedVespa(isHostedVespa));
+    }
+
+    private static ContentCluster createOneNodeCluster(TestProperties props) throws Exception {
+        return createOneNodeCluster(oneNodeClusterXml(), props);
+    }
+
+    private static ContentCluster createOneNodeCluster(TestProperties props, Optional<Flavor> flavor) throws Exception {
+        return createOneNodeCluster(oneNodeClusterXml(), props, flavor);
     }
 
     private static ContentCluster createClusterWithFlushOnShutdownOverride(boolean flushOnShutdown, boolean isHostedVespa) throws Exception {
@@ -818,13 +833,20 @@ public class ContentClusterTest extends ContentBaseTest {
                 "  <group>" +
                 "    <node distribution-key=\"0\" hostalias=\"mockhost\"/>" +
                 "  </group>" +
-                "</content>", isHostedVespa);
+                "</content>", new TestProperties().setHostedVespa(isHostedVespa));
     }
 
-    private static ContentCluster createOneNodeCluster(String clusterXml, boolean isHostedVespa) throws Exception {
+    private static ContentCluster createOneNodeCluster(String clusterXml, TestProperties props) throws Exception {
+        return createOneNodeCluster(clusterXml, props, Optional.empty());
+    }
+
+    private static ContentCluster createOneNodeCluster(String clusterXml, TestProperties props, Optional<Flavor> flavor) throws Exception {
         DeployState.Builder deployStateBuilder = new DeployState.Builder()
-                .properties(new TestProperties().setHostedVespa(isHostedVespa));
-        MockRoot root = ContentClusterUtils.createMockRoot(Collections.emptyList(), deployStateBuilder);
+                .properties(props);
+        MockRoot root = flavor.isPresent() ?
+                ContentClusterUtils.createMockRoot(new SingleNodeProvisioner(flavor.get()),
+                        Collections.emptyList(), deployStateBuilder) :
+                ContentClusterUtils.createMockRoot(Collections.emptyList(), deployStateBuilder);
         ContentCluster cluster = ContentClusterUtils.createCluster(clusterXml, root);
         root.freezeModelTopology();
         cluster.validate();
@@ -1015,41 +1037,58 @@ public class ContentClusterTest extends ContentBaseTest {
         }
     }
 
-    private StorDistributormanagerConfig resolveStorDistributormanagerConfig(TestProperties props) {
-        VespaModel model = createEnd2EndOneNode(props);
+    private StorDistributormanagerConfig resolveStorDistributormanagerConfig(TestProperties props) throws Exception {
+        var cc = createOneNodeCluster(props);
 
-        ContentCluster cc = model.getContentClusters().get("storage");
         var builder = new StorDistributormanagerConfig.Builder();
         cc.getDistributorNodes().getConfig(builder);
 
         return (new StorDistributormanagerConfig(builder));
     }
 
-    private int resolveMaxInhibitedGroupsConfigWithFeatureFlag(int maxGroups) {
+    private int resolveMaxInhibitedGroupsConfigWithFeatureFlag(int maxGroups) throws Exception {
         var cfg = resolveStorDistributormanagerConfig(new TestProperties().maxActivationInhibitedOutOfSyncGroups(maxGroups));
         return cfg.max_activation_inhibited_out_of_sync_groups();
     }
 
     @Test
-    public void default_distributor_max_inhibited_group_activation_config_controlled_by_properties() {
+    public void default_distributor_max_inhibited_group_activation_config_controlled_by_properties() throws Exception {
         assertEquals(0, resolveMaxInhibitedGroupsConfigWithFeatureFlag(0));
         assertEquals(2, resolveMaxInhibitedGroupsConfigWithFeatureFlag(2));
     }
 
-    private int resolveNumDistributorStripesConfigWithFeatureFlag(TestProperties props) {
-        var cfg = resolveStorDistributormanagerConfig(props);
-        return cfg.num_distributor_stripes();
+    private int resolveNumDistributorStripesConfigWithFeatureFlag(TestProperties props, Optional<Flavor> flavor) throws Exception {
+        var cc = createOneNodeCluster(props, flavor);
+        var builder = new StorDistributormanagerConfig.Builder();
+        cc.getDistributorNodes().getChildren().get("0").getConfig(builder);
+        return (new StorDistributormanagerConfig(builder)).num_distributor_stripes();
     }
 
-    private int resolveNumDistributorStripesConfigWithFeatureFlag(int numStripes) {
-        return resolveNumDistributorStripesConfigWithFeatureFlag(new TestProperties().setNumDistributorStripes(numStripes));
+    private int resolveNumDistributorStripesConfigWithFeatureFlag(int numStripes) throws Exception {
+        return resolveNumDistributorStripesConfigWithFeatureFlag(new TestProperties().setNumDistributorStripes(numStripes), Optional.empty());
+    }
+
+    private int resolveTunedNumDistributorStripesConfig(int numCpuCores) throws Exception {
+        var flavor = new Flavor(new FlavorsConfig.Flavor(new FlavorsConfig.Flavor.Builder().name("test").minCpuCores(numCpuCores)));
+        return resolveNumDistributorStripesConfigWithFeatureFlag(new TestProperties().setNumDistributorStripes(-1),
+                Optional.of(flavor));
     }
 
     @Test
-    public void num_distributor_stripes_config_controlled_by_properties() {
-        assertEquals(0, resolveNumDistributorStripesConfigWithFeatureFlag(new TestProperties()));
+    public void num_distributor_stripes_config_controlled_by_properties() throws Exception {
+        assertEquals(0, resolveNumDistributorStripesConfigWithFeatureFlag(new TestProperties(), Optional.empty()));
         assertEquals(0, resolveNumDistributorStripesConfigWithFeatureFlag(0));
         assertEquals(1, resolveNumDistributorStripesConfigWithFeatureFlag(1));
+        assertEquals(4, resolveNumDistributorStripesConfigWithFeatureFlag(4));
+    }
+
+    @Test
+    public void num_distributor_stripes_config_tuned_by_flavor() throws Exception {
+        assertEquals(1, resolveTunedNumDistributorStripesConfig(1));
+        assertEquals(1, resolveTunedNumDistributorStripesConfig(16));
+        assertEquals(2, resolveTunedNumDistributorStripesConfig(17));
+        assertEquals(2, resolveTunedNumDistributorStripesConfig(64));
+        assertEquals(4, resolveTunedNumDistributorStripesConfig(65));
     }
 
     @Test
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 a0b88b531ab..ac4a989c89b 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
@@ -35,7 +35,7 @@ public class ContentClusterUtils {
         return createMockRoot(provisioner, schemas, new DeployState.Builder());
     }
 
-    private static MockRoot createMockRoot(HostProvisioner provisioner, List<String> schemas, DeployState.Builder deployStateBuilder) {
+    public static MockRoot createMockRoot(HostProvisioner provisioner, List<String> schemas, DeployState.Builder deployStateBuilder) {
         ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withSchemas(schemas).build();
         DeployState deployState = deployStateBuilder.applicationPackage(applicationPackage)
                           .modelHostProvisioner(provisioner)