Merge pull request #13204 from vespa-engine/balder/control-num-response-threads

Control num response threads with flag

7 files changed, 89 insertions, 19 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index 394018e45db..7641f14b007 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -70,6 +70,9 @@ public interface ModelContext {
         double defaultTermwiseLimit();
 
         // TODO Revisit in May or June 2020
+        int defaultNumResponseThreads();
+
+        // TODO Revisit in May or June 2020
         double threadPoolSizeFactor();
 
         // TODO Revisit in May or June 2020
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 55c3f98f0f3..4dbfc8f7a8f 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
@@ -46,6 +46,7 @@ public class TestProperties implements ModelContext.Properties {
     private double softStartSeconds = 0.0;
     private double threadPoolSizeFactor = 0.0;
     private double queueSizeFactor = 0.0;
+    private int defaultNumResponseThreads = 0;
     private Optional<EndpointCertificateSecrets> endpointCertificateSecrets = Optional.empty();
     private AthenzDomain athenzDomain;
 
@@ -81,6 +82,10 @@ public class TestProperties implements ModelContext.Properties {
         return softStartSeconds;
     }
 
+    @Override public int defaultNumResponseThreads() {
+        return defaultNumResponseThreads;
+    }
+
     @Override public double defaultTopKProbability() { return topKProbability; }
     @Override public boolean useDistributorBtreeDb() { return useDistributorBtreeDb; }
     @Override public boolean useThreePhaseUpdates() { return useThreePhaseUpdates; }
@@ -101,6 +106,11 @@ public class TestProperties implements ModelContext.Properties {
         return this;
     }
 
+    public TestProperties setDefaultNumResponseThreads(int numResponseThreads) {
+        defaultNumResponseThreads = numResponseThreads;
+        return this;
+    }
+
     public TestProperties setUseThreePhaseUpdates(boolean useThreePhaseUpdates) {
         this.useThreePhaseUpdates = useThreePhaseUpdates;
         return this;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java
index 785088e96fc..76c05d4028a 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/FileStorProducer.java
@@ -11,8 +11,8 @@ import com.yahoo.vespa.model.content.cluster.ContentCluster;
 public class FileStorProducer implements StorFilestorConfig.Producer {
 
     public static class Builder {
-        protected FileStorProducer build(ContentCluster parent, ModelElement clusterElem) {
-            return new FileStorProducer(parent, getThreads(clusterElem));
+        protected FileStorProducer build(ContentCluster parent, Integer numResponseThreads, ModelElement clusterElem) {
+            return new FileStorProducer(parent, getThreads(clusterElem), numResponseThreads);
         }
 
        private Integer getThreads(ModelElement clusterElem) {
@@ -41,12 +41,14 @@ public class FileStorProducer implements StorFilestorConfig.Producer {
        }
     }
 
-    private Integer numThreads;
-    private ContentCluster cluster;
+    private final Integer numThreads;
+    private final ContentCluster cluster;
+    private final Integer numResponseThreads;
 
-    public FileStorProducer(ContentCluster parent, Integer numThreads) {
+    public FileStorProducer(ContentCluster parent, Integer numThreads, Integer numResponseThreads) {
         this.numThreads = numThreads;
         this.cluster = parent;
+        this.numResponseThreads = numResponseThreads;
     }
 
     @Override
@@ -54,6 +56,9 @@ public class FileStorProducer implements StorFilestorConfig.Producer {
         if (numThreads != null) {
             builder.num_threads(numThreads);
         }
+        if (numResponseThreads != null) {
+            builder.num_response_threads(numResponseThreads);
+        }
         builder.enable_multibit_split_optimalization(cluster.getPersistence().enableMultiLevelSplitting());
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java
index 091e82a9c76..c97b4c50484 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java
@@ -36,7 +36,7 @@ public class StorageCluster extends AbstractConfigProducer<StorageNode>
 
             return new StorageCluster(ancestor,
                                       ContentCluster.getClusterName(clusterElem),
-                                      new FileStorProducer.Builder().build(cluster, clusterElem),
+                                      new FileStorProducer.Builder().build(cluster, deployState.getProperties().defaultNumResponseThreads(), clusterElem),
                                       new IntegrityCheckerProducer.Builder().build(cluster, clusterElem),
                                       new StorServerProducer.Builder().build(clusterElem),
                                       new StorVisitorProducer.Builder().build(clusterElem),
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java
index 883d8b89765..fdf911231d5 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java
@@ -1,6 +1,9 @@
 // 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.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.MockApplicationPackage;
 import com.yahoo.config.provision.Flavor;
@@ -20,16 +23,28 @@ import org.junit.Test;
 
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public class StorageClusterTest {
 
-    StorageCluster parse(String xml, Flavor flavor) throws Exception {
-        MockRoot root = new MockRoot("", new MockApplicationPackage.Builder().build(),
-                new SingleNodeProvisioner(flavor));
+    StorageCluster parse(String xml, Flavor flavor) {
+        MockRoot root = new MockRoot("", new DeployState.Builder()
+                .applicationPackage(new MockApplicationPackage.Builder().build())
+                .modelHostProvisioner(new SingleNodeProvisioner(flavor)).build());
         return parse(xml, root);
     }
-    StorageCluster parse(String xml) throws Exception {
+
+    StorageCluster parse(String xml, Flavor flavor, ModelContext.Properties properties) {
+        MockRoot root = new MockRoot("", new DeployState.Builder()
+                .applicationPackage(new MockApplicationPackage.Builder().build())
+                .modelHostProvisioner(new SingleNodeProvisioner(flavor))
+                .properties(properties).build());
+        return parse(xml, root);
+    }
+
+    StorageCluster parse(String xml) {
         MockRoot root = new MockRoot();
         return parse(xml, root);
     }
@@ -47,7 +62,7 @@ public class StorageClusterTest {
     }
 
     @Test
-    public void testBasics() throws Exception {
+    public void testBasics() {
         StorServerConfig.Builder builder = new StorServerConfig.Builder();
         parse("<content id=\"foofighters\"><documents/>\n" +
               "  <group>" +
@@ -62,7 +77,7 @@ public class StorageClusterTest {
     }
 
     @Test
-    public void testMerges() throws Exception {
+    public void testMerges() {
         StorServerConfig.Builder builder = new StorServerConfig.Builder();
         parse("" +
                 "<content id=\"foofighters\">\n" +
@@ -106,7 +121,7 @@ public class StorageClusterTest {
     }
 
     @Test
-    public void testPersistenceThreads() throws Exception {
+    public void testPersistenceThreads() {
 
         StorageCluster stc = parse(
                 "<cluster id=\"bees\">\n" +
@@ -127,7 +142,8 @@ public class StorageClusterTest {
             StorFilestorConfig config = new StorFilestorConfig(builder);
 
             assertEquals(7, config.num_threads());
-            assertEquals(false, config.enable_multibit_split_optimalization());
+            assertFalse(config.enable_multibit_split_optimalization());
+            assertEquals(0, config.num_response_threads());
         }
         {
             assertEquals(1, stc.getChildren().size());
@@ -140,7 +156,29 @@ public class StorageClusterTest {
     }
 
     @Test
-    public void testPersistenceThreadsOld() throws Exception {
+    public void testResponseThreads() {
+
+        StorageCluster stc = parse(
+                "<cluster id=\"bees\">\n" +
+                        "    <documents/>" +
+                        "    <tuning>\n" +
+                        "        <persistence-threads count=\"7\"/>\n" +
+                        "    </tuning>\n" +
+                        "  <group>" +
+                        "     <node distribution-key=\"0\" hostalias=\"mockhost\"/>" +
+                        "  </group>" +
+                        "</cluster>",
+                new Flavor(new FlavorsConfig.Flavor.Builder().name("test-flavor").minCpuCores(9).build()),
+                new TestProperties().setDefaultNumResponseThreads(3)
+        );
+        StorFilestorConfig.Builder builder = new StorFilestorConfig.Builder();
+        stc.getConfig(builder);
+        StorFilestorConfig config = new StorFilestorConfig(builder);
+        assertEquals(3, config.num_response_threads());
+    }
+
+    @Test
+    public void testPersistenceThreadsOld() {
 
         StorageCluster stc = parse(
                 "<cluster id=\"bees\">\n" +
@@ -165,7 +203,7 @@ public class StorageClusterTest {
             StorFilestorConfig config = new StorFilestorConfig(builder);
 
             assertEquals(4, config.num_threads());
-            assertEquals(false, config.enable_multibit_split_optimalization());
+            assertFalse(config.enable_multibit_split_optimalization());
         }
         {
             assertEquals(1, stc.getChildren().size());
@@ -178,7 +216,7 @@ public class StorageClusterTest {
     }
 
     @Test
-    public void testNoPersistenceThreads() throws Exception {
+    public void testNoPersistenceThreads() {
         StorageCluster stc = parse(
                 "<cluster id=\"bees\">\n" +
                         "    <documents/>" +
@@ -208,7 +246,7 @@ public class StorageClusterTest {
     }
 
     @Test
-    public void integrity_checker_explicitly_disabled_when_not_running_with_vds_provider() throws Exception {
+    public void integrity_checker_explicitly_disabled_when_not_running_with_vds_provider() {
         StorIntegritycheckerConfig.Builder builder = new StorIntegritycheckerConfig.Builder();
         parse(
                 "<cluster id=\"bees\">\n" +
@@ -299,7 +337,7 @@ public class StorageClusterTest {
         cluster.getStorageNodes().getConfig(builder);
 
         PersistenceConfig config = new PersistenceConfig(builder);
-        assertEquals(true, config.fail_partition_on_error());
+        assertTrue(config.fail_partition_on_error());
         assertEquals(34 * 60, config.revert_time_period());
         assertEquals(5 * 24 * 60 * 60, config.keep_remove_time_period());
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index dfc71c7fd9f..fa7a107f953 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -154,6 +154,7 @@ public class ModelContextImpl implements ModelContext {
         private final double defaultSoftStartSeconds;
         private final double threadPoolSizeFactor;
         private final double queueSizefactor;
+        private final int defaultNumResponseThreads;
         private final Optional<AthenzDomain> athenzDomain;
 
         public Properties(ApplicationId applicationId,
@@ -198,6 +199,8 @@ public class ModelContextImpl implements ModelContext {
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
             queueSizefactor = Flags.DEFAULT_QUEUE_SIZE_FACTOR.bindTo(flagSource)
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
+            defaultNumResponseThreads = Flags.DEFAULT_NUM_RESPONSE_THREADS.bindTo(flagSource)
+                    .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
             this.athenzDomain = athenzDomain;
         }
 
@@ -277,6 +280,11 @@ public class ModelContextImpl implements ModelContext {
         }
 
         @Override
+        public int defaultNumResponseThreads() {
+            return defaultNumResponseThreads;
+        }
+
+        @Override
         public Optional<AthenzDomain> athenzDomain() { return athenzDomain; }
 
     }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 071fc3844ba..20e3216166f 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -160,6 +160,12 @@ public class Flags {
             "Takes effect at redeployment",
             ZONE_ID, APPLICATION_ID);
 
+    public static final UnboundIntFlag DEFAULT_NUM_RESPONSE_THREADS = defineIntFlag(
+            "default-num-response-threads", 0,
+            "Default number of threads used for processing put/update/remove responses.",
+            "Takes effect at redeployment",
+            ZONE_ID, APPLICATION_ID);
+
     public static final UnboundBooleanFlag USE_DISTRIBUTOR_BTREE_DB = defineFeatureFlag(
             "use-distributor-btree-db", false,
             "Whether to use the new B-tree bucket database in the distributors.",