Merge pull request #20823 from vespa-engine/balder/only-2-threads-for-cc

All non application clusters only needs a few threads. 2 should be su…
vespa-engine · Jan 16, 2022 · c318c14 · c318c14
2 parents 694f3e7 + 9a4b0b1
commit c318c14
Show file tree

Hide file tree

Showing 10 changed files with 19 additions and 31 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
@@ -82,7 +82,8 @@ interface FeatureFlags {
         @ModelFeatureFlag(owners = {"baldersheim"}) default boolean skipMbusReplyThread() { throw new UnsupportedOperationException("TODO specify default value"); }
         @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useAsyncMessageHandlingOnSchedule() { throw new UnsupportedOperationException("TODO specify default value"); }
         @ModelFeatureFlag(owners = {"baldersheim"}) default double feedConcurrency() { throw new UnsupportedOperationException("TODO specify default value"); }
-        @ModelFeatureFlag(owners = {"baldersheim"}) default int metricsproxyNumThreads() { throw new UnsupportedOperationException("TODO specify default value"); }
+        @ModelFeatureFlag(owners = {"baldersheim"}, removeAfter = "7.527") default int metricsproxyNumThreads() { return defaultPoolNumThreads(); }
+        @ModelFeatureFlag(owners = {"baldersheim"}) default int defaultPoolNumThreads() { return 2; }
         @ModelFeatureFlag(owners = {"baldersheim"}, removeAfter = "7.527") default int largeRankExpressionLimit() { return 8192; }
         @ModelFeatureFlag(owners = {"baldersheim"}) default int maxUnCommittedMemory() { return 130000; }
         @ModelFeatureFlag(owners = {"baldersheim"}) default int maxConcurrentMergesPerNode() { return 16; }

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
@@ -118,7 +118,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public double resourceLimitDisk() { return resourceLimitDisk; }
     @Override public double resourceLimitMemory() { return resourceLimitMemory; }
     @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
-    @Override public int metricsproxyNumThreads() { return 1; }
     @Override public double containerShutdownTimeout() { return containerShutdownTimeout; }
     @Override public boolean containerDumpHeapOnShutdownTimeout() { return containerDumpHeapOnShutdownTimeout; }
     @Override public int distributorMergeBusyWait() { return distributorMergeBusyWait; }

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
@@ -17,7 +17,7 @@
 public class LogserverContainerCluster extends ContainerCluster<LogserverContainer> {
 
     public LogserverContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) {
-        super(parent, name, name, deployState, true);
+        super(parent, name, name, deployState, true, deployState.featureFlags().defaultPoolNumThreads());
 
         addDefaultHandlersWithVip();
         addLogHandler();

diff --git a/...java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/...java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
@@ -22,7 +22,7 @@ public class ClusterControllerContainerCluster extends ContainerCluster<ClusterC
 
     public ClusterControllerContainerCluster(
             AbstractConfigProducer<?> parent, String subId, String name, DeployState deployState) {
-        super(parent, subId, name, deployState, false);
+        super(parent, subId, name, deployState, false, deployState.featureFlags().defaultPoolNumThreads());
         addDefaultHandlersWithVip();
         this.reindexingContext = createReindexingContext(deployState);
         setJvmGCOptions(deployState.getProperties().jvmGCOptions(Optional.of(ClusterSpec.Type.admin)));

diff --git a/.../src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/.../src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
@@ -87,7 +87,7 @@ static final class AppDimensionNames {
     private final ApplicationId applicationId;
 
     public MetricsProxyContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) {
-        super(parent, name, name, deployState, true);
+        super(parent, name, name, deployState, true, deployState.featureFlags().defaultPoolNumThreads());
         this.parent = parent;
         applicationId = deployState.getProperties().applicationId();
 

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
@@ -94,7 +94,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
     private List<ApplicationClusterEndpoint> endpointList = List.of();
 
     public ApplicationContainerCluster(AbstractConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState) {
-        super(parent, configSubId, clusterId, deployState, true);
+        super(parent, configSubId, clusterId, deployState, true, 10);
         this.tlsClientAuthority = deployState.tlsClientAuthority();
         previousHosts = deployState.getPreviousModel().stream()
                                    .map(Model::allocatedHosts)

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
@@ -160,7 +160,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     private boolean deferChangesUntilRestart = false;
 
-    public ContainerCluster(AbstractConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState, boolean zooKeeperLocalhostAffinity) {
+    public ContainerCluster(AbstractConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState, boolean zooKeeperLocalhostAffinity, int defaultPoolNumThreads) {
         super(parent, configSubId);
         this.name = clusterId;
         this.isHostedVespa = stateIsHosted(deployState);
@@ -176,7 +176,7 @@ public ContainerCluster(AbstractConfigProducer<?> parent, String configSubId, St
 
         addComponent(new StatisticsComponent());
         addSimpleComponent(AccessLog.class);
-        addComponent(new DefaultThreadpoolProvider(this, deployState.featureFlags().metricsproxyNumThreads()));
+        addComponent(new DefaultThreadpoolProvider(this, defaultPoolNumThreads));
         addSimpleComponent(com.yahoo.concurrent.classlock.ClassLocking.class);
         addSimpleComponent("com.yahoo.container.jdisc.metric.MetricConsumerProviderProvider");
         addSimpleComponent("com.yahoo.container.jdisc.metric.MetricProvider");

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/DefaultThreadpoolProvider.java b/config-model/src/main/java/com/yahoo/vespa/model/container/DefaultThreadpoolProvider.java
@@ -5,7 +5,6 @@
 import com.yahoo.container.handler.ThreadPoolProvider;
 import com.yahoo.container.handler.ThreadpoolConfig;
 import com.yahoo.osgi.provider.model.ComponentModel;
-import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster;
 import com.yahoo.vespa.model.container.component.SimpleComponent;
 
 /**
@@ -16,38 +15,27 @@
 class DefaultThreadpoolProvider extends SimpleComponent implements ThreadpoolConfig.Producer {
 
     private final ContainerCluster<?> cluster;
-    private final int metricsproxyNumThreads;
+    private final int defaultWorkerThreads;
 
-    DefaultThreadpoolProvider(ContainerCluster<?> cluster, int metricsproxyNumThreads) {
+    DefaultThreadpoolProvider(ContainerCluster<?> cluster, int defaultWorkerThreads) {
         super(new ComponentModel(
                 BundleInstantiationSpecification.getFromStrings(
                         "default-threadpool",
                         ThreadPoolProvider.class.getName(),
                         null)));
         this.cluster = cluster;
-        this.metricsproxyNumThreads = metricsproxyNumThreads;
-    }
-
-    private int defaultThreadsByClusterType() {
-        if (cluster instanceof MetricsProxyContainerCluster) {
-            return metricsproxyNumThreads;
-        }
-        return 10;
+        this.defaultWorkerThreads = defaultWorkerThreads;
     }
 
     @Override
     public void getConfig(ThreadpoolConfig.Builder builder) {
-        if (!(cluster instanceof ApplicationContainerCluster)) {
+        if (cluster instanceof ApplicationContainerCluster) {
+            // Core pool size of 2xcores, and max of 100xcores and using a synchronous Q
+            // This is the deafault pool used by both federation and generally when you ask for an Executor.
+            builder.corePoolSize(-2).maxthreads(-100).queueSize(0);
+        } else {
             // Container clusters such as logserver, metricsproxy and clustercontroller
-            int defaultWorkerThreads = defaultThreadsByClusterType();
-            builder.maxthreads(defaultWorkerThreads);
-            builder.corePoolSize(defaultWorkerThreads);
-            builder.queueSize(50);
-            return;
+            builder.corePoolSize(defaultWorkerThreads).maxthreads(defaultWorkerThreads).queueSize(50);
         }
-
-        // Core pool size of 2xcores, and max of 100xcores and using a synchronous Q
-        // This is the deafault pool used by both federation and generally when you ask for an Executor.
-        builder.corePoolSize(-2).maxthreads(-100).queueSize(0);
     }
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
@@ -190,7 +190,7 @@ public void testClusterControllerResourceUsage() {
         root.freezeModelTopology();
 
         ThreadpoolConfig threadpoolConfig = root.getConfig(ThreadpoolConfig.class, "container0/component/default-threadpool");
-        assertEquals(10, threadpoolConfig.maxthreads());
+        assertEquals(2, threadpoolConfig.maxthreads());
         assertEquals(50, threadpoolConfig.queueSize());
     }
 

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
@@ -273,7 +273,7 @@ public FeatureFlags(FlagSource source, ApplicationId appId) {
         @Override public double resourceLimitDisk() { return resourceLimitDisk; }
         @Override public double resourceLimitMemory() { return resourceLimitMemory; }
         @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
-        @Override public int metricsproxyNumThreads() { return metricsproxyNumThreads; }
+        @Override public int defaultPoolNumThreads() { return metricsproxyNumThreads; }
         @Override public double containerShutdownTimeout() { return containerShutdownTimeout; }
         @Override public boolean containerDumpHeapOnShutdownTimeout() { return containerDumpHeapOnShutdownTimeout; }
         @Override public int distributorMergeBusyWait() { return distributorMergeBusyWait; }