Inject a default threadpool for all handlers that don't declare their own.

6 files changed, 94 insertions, 53 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
index 33c125dcecf..366592d98e6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.model.clients;
 
 import com.yahoo.config.model.producer.AbstractConfigProducer;
-import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig;
 import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.vespa.model.container.ContainerCluster;
@@ -41,31 +40,31 @@ public class ContainerDocumentApi {
 
     private static void addFeedHandler(ContainerCluster<?> cluster, HandlerOptions handlerOptions) {
         String bindingSuffix = ContainerCluster.RESERVED_URI_PREFIX + "/feedapi";
-        var handler = newVespaClientHandler("com.yahoo.vespa.http.server.FeedHandler", bindingSuffix, handlerOptions);
-        cluster.addComponent(handler);
         var executor = new Threadpool("feedapi-handler", handlerOptions.feedApiThreadpoolOptions);
-        handler.inject(executor);
-        handler.addComponent(executor);
+        var handler = newVespaClientHandler("com.yahoo.vespa.http.server.FeedHandler",
+                                            bindingSuffix, handlerOptions, executor);
+        cluster.addComponent(handler);
     }
 
 
     private static void addRestApiHandler(ContainerCluster<?> cluster, HandlerOptions handlerOptions) {
-        var handler = newVespaClientHandler("com.yahoo.document.restapi.resource.DocumentV1ApiHandler", DOCUMENT_V1_PREFIX + "/*", handlerOptions);
+        var handler = newVespaClientHandler("com.yahoo.document.restapi.resource.DocumentV1ApiHandler",
+                                            DOCUMENT_V1_PREFIX + "/*", handlerOptions, null);
         cluster.addComponent(handler);
 
         // We need to include a dummy implementation of the previous restapi handler (using the same class name).
         // The internal legacy test framework requires that the name of the old handler is listed in /ApplicationStatus.
-        var oldHandlerDummy = handlerComponentSpecification("com.yahoo.document.restapi.resource.RestApi");
+        var oldHandlerDummy = createHandler("com.yahoo.document.restapi.resource.RestApi", null);
         cluster.addComponent(oldHandlerDummy);
     }
 
     public boolean ignoreUndefinedFields() { return ignoreUndefinedFields; }
 
-    private static Handler<AbstractConfigProducer<?>> newVespaClientHandler(
-            String componentId,
-            String bindingSuffix,
-            HandlerOptions handlerOptions) {
-        Handler<AbstractConfigProducer<?>> handler = handlerComponentSpecification(componentId);
+    private static Handler<AbstractConfigProducer<?>> newVespaClientHandler(String componentId,
+                                                                            String bindingSuffix,
+                                                                            HandlerOptions handlerOptions,
+                                                                            Threadpool executor) {
+        Handler<AbstractConfigProducer<?>> handler = createHandler(componentId, executor);
         if (handlerOptions.bindings.isEmpty()) {
             handler.addServerBindings(
                     SystemBindingPattern.fromHttpPath(bindingSuffix),
@@ -81,9 +80,9 @@ public class ContainerDocumentApi {
         return handler;
     }
 
-    private static Handler<AbstractConfigProducer<?>> handlerComponentSpecification(String className) {
-        return new Handler<>(new ComponentModel(
-                BundleInstantiationSpecification.getFromStrings(className, null, "vespaclient-container-plugin"), ""));
+    private static Handler<AbstractConfigProducer<?>> createHandler(String className, Threadpool executor) {
+        return new Handler<>(new ComponentModel(className, null, "vespaclient-container-plugin"),
+                             executor);
     }
 
     public static final class HandlerOptions {
@@ -104,12 +103,10 @@ public class ContainerDocumentApi {
         }
 
         @Override
-        public void getConfig(ContainerThreadpoolConfig.Builder builder) {
-            super.getConfig(builder);
-
-            // User options overrides below configuration
-            if (hasUserOptions()) return;
-            builder.maxThreads(-4).minThreads(-4).queueSize(500);
+        protected void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder) {
+            builder.maxThreads(-4)
+                    .minThreads(-4)
+                    .queueSize(500);
         }
     }
 
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
index 2385b5b3812..d98f04e80ba 100755
--- 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
@@ -142,6 +142,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
     private ContainerDocproc containerDocproc;
     private ContainerDocumentApi containerDocumentApi;
     private SecretStore secretStore;
+    private final ContainerThreadpool defaultHandlerThreadpool = new Handler.DefaultHandlerThreadpool();
 
     private boolean rpcServerEnabled = true;
     private boolean httpServerEnabled = true;
@@ -181,6 +182,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         addCommonVespaBundles();
         addSimpleComponent(AccessLog.class);
         addComponent(new DefaultThreadpoolProvider(this, defaultPoolNumThreads));
+        addComponent(defaultHandlerThreadpool);
         addSimpleComponent(com.yahoo.concurrent.classlock.ClassLocking.class);
         addSimpleComponent("com.yahoo.container.jdisc.metric.MetricConsumerProviderProvider");
         addSimpleComponent("com.yahoo.container.jdisc.metric.MetricProvider");
@@ -247,6 +249,15 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     public final void addComponent(Component<?, ?> component) {
         componentGroup.addComponent(component);
+        if (component instanceof Handler<?> handler) {
+            ensureHandlerHasThreadpool(handler);
+        }
+    }
+
+    private void ensureHandlerHasThreadpool(Handler<?> handler) {
+        if (! handler.hasCustomThreadPool) {
+            handler.inject(defaultHandlerThreadpool);
+        }
     }
 
     public final void addSimpleComponent(String idSpec, String classSpec, String bundleSpec) {
@@ -306,10 +317,8 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         this.processingChains = processingChains;
 
         // Cannot use the class object for ProcessingHandler, because its superclass is not accessible
-        ProcessingHandler<?> processingHandler = new ProcessingHandler<>(
-                processingChains,
-                "com.yahoo.processing.handler.ProcessingHandler",
-                 null);
+        ProcessingHandler<?> processingHandler = new ProcessingHandler<>(processingChains,
+                                                                         "com.yahoo.processing.handler.ProcessingHandler");
 
         for (BindingPattern binding: serverBindings)
             processingHandler.addServerBindings(binding);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java
index 489e4cc135a..fbd7bc9fe56 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java
@@ -17,7 +17,7 @@ import java.util.Optional;
  *
  * @author bjorncs
  */
-public class ContainerThreadpool extends SimpleComponent implements ContainerThreadpoolConfig.Producer {
+public abstract class ContainerThreadpool extends SimpleComponent implements ContainerThreadpoolConfig.Producer {
 
     private final String name;
     private final UserOptions userOptions;
@@ -32,8 +32,13 @@ public class ContainerThreadpool extends SimpleComponent implements ContainerThr
         this.userOptions = userOptions;
     }
 
+    // Must be implemented by subclasses to set values that may be overridden by user options.
+    protected abstract void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder);
+
     @Override
     public void getConfig(ContainerThreadpoolConfig.Builder builder) {
+        setDefaultConfigValues(builder);
+
         builder.name(this.name);
         if (userOptions != null) {
             builder.maxThreads(userOptions.maxThreads);
@@ -42,9 +47,6 @@ public class ContainerThreadpool extends SimpleComponent implements ContainerThr
         }
     }
 
-    protected Optional<UserOptions> userOptions() { return Optional.ofNullable(userOptions); }
-    protected boolean hasUserOptions() { return userOptions().isPresent(); }
-
     public static class UserOptions {
         private final int maxThreads;
         private final int minThreads;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/Handler.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/Handler.java
index 8ffdccae896..39687ad6459 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/Handler.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/Handler.java
@@ -2,7 +2,9 @@
 package com.yahoo.vespa.model.container.component;
 
 import com.yahoo.config.model.producer.AbstractConfigProducer;
+import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig;
 import com.yahoo.osgi.provider.model.ComponentModel;
+import com.yahoo.vespa.model.container.ContainerThreadpool;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -25,8 +27,24 @@ public class Handler<CHILD extends AbstractConfigProducer<?>> extends Component<
     private final Set<BindingPattern> serverBindings = new LinkedHashSet<>();
     private final List<BindingPattern> clientBindings = new ArrayList<>();
 
+    public final boolean hasCustomThreadPool;
+
     public Handler(ComponentModel model) {
+        this(model, null);
+    }
+
+    @SuppressWarnings("unchecked")
+    public Handler(ComponentModel model, ContainerThreadpool threadpool) {
         super(model);
+
+        // The default threadpool is always added to the cluster, so cannot be added here.
+        if (threadpool != null) {
+            hasCustomThreadPool = true;
+            addComponent((CHILD) threadpool);
+            inject(threadpool);
+        } else {
+            hasCustomThreadPool = false;
+        }
     }
 
     public static Handler<AbstractConfigProducer<?>> fromClassName(String className) {
@@ -53,4 +71,24 @@ public class Handler<CHILD extends AbstractConfigProducer<?>> extends Component<
         return Collections.unmodifiableList(clientBindings);
     }
 
+
+    /**
+     * The default threadpool for all handlers, except those that declare their own, e.g. SearchHandler.
+     */
+    public static class DefaultHandlerThreadpool extends ContainerThreadpool {
+
+        public DefaultHandlerThreadpool() {
+            super("default-handler-common", null);
+        }
+
+        @Override
+        public void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder) {
+            builder.maxThreadExecutionTimeSeconds(190)
+                    .keepAliveTime(5.0)
+                    .maxThreads(-2)
+                    .minThreads(-2)
+                    .queueSize(-40);
+        }
+    }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/ProcessingHandler.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/ProcessingHandler.java
index 3f68a0a2709..34531a655f7 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/ProcessingHandler.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/ProcessingHandler.java
@@ -5,6 +5,8 @@ import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.core.ChainsConfig;
 import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
+import com.yahoo.vespa.model.container.Container;
+import com.yahoo.vespa.model.container.ContainerThreadpool;
 import com.yahoo.vespa.model.container.component.Handler;
 
 
@@ -20,15 +22,11 @@ public class ProcessingHandler<CHAINS extends Chains<?>>
     protected final CHAINS chains;
 
     public ProcessingHandler(CHAINS chains, String handlerClass) {
-        this(chains, BundleInstantiationSpecification.getInternalProcessingSpecificationFromStrings(handlerClass, null));
+        this(chains, BundleInstantiationSpecification.getInternalProcessingSpecificationFromStrings(handlerClass, null), null);
     }
 
-    public ProcessingHandler(CHAINS chains, String handlerClass, String bundle) {
-        this(chains, BundleInstantiationSpecification.getFromStrings(handlerClass, null, bundle));
-    }
-
-    private ProcessingHandler(CHAINS chains, BundleInstantiationSpecification spec) {
-        super(new ComponentModel(spec, null));
+    public ProcessingHandler(CHAINS chains, BundleInstantiationSpecification spec, ContainerThreadpool threadpool) {
+        super(new ComponentModel(spec, null), threadpool);
         this.chains = chains;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java
index 54d943f498a..6b0bf8a67b9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.container.xml;
 
+import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig;
 import com.yahoo.vespa.model.container.ApplicationContainerCluster;
 import com.yahoo.vespa.model.container.ContainerThreadpool;
@@ -11,6 +12,8 @@ import com.yahoo.vespa.model.container.search.searchchain.SearchChains;
 
 import java.util.List;
 
+import static com.yahoo.container.bundle.BundleInstantiationSpecification.getInternalHandlerSpecificationFromStrings;
+
 /**
  * Component definition for {@link com.yahoo.search.handler.SearchHandler}
  *
@@ -19,38 +22,32 @@ import java.util.List;
 class SearchHandler extends ProcessingHandler<SearchChains> {
 
     static final String HANDLER_CLASS = com.yahoo.search.handler.SearchHandler.class.getName();
+    static final BundleInstantiationSpecification HANDLER_SPEC = getInternalHandlerSpecificationFromStrings(HANDLER_CLASS, null);
     static final BindingPattern DEFAULT_BINDING = SystemBindingPattern.fromHttpPath("/search/*");
 
     SearchHandler(ApplicationContainerCluster cluster,
                   List<BindingPattern> bindings,
                   ContainerThreadpool.UserOptions threadpoolOptions) {
-        super(cluster.getSearchChains(), HANDLER_CLASS);
+        super(cluster.getSearchChains(), HANDLER_SPEC, new Threadpool(threadpoolOptions));
         bindings.forEach(this::addServerBindings);
-        Threadpool threadpool = new Threadpool(cluster, threadpoolOptions);
-        inject(threadpool);
-        addComponent(threadpool);
     }
 
+
     private static class Threadpool extends ContainerThreadpool {
-        private final ApplicationContainerCluster cluster;
 
-        Threadpool(ApplicationContainerCluster cluster, UserOptions options) {
+        Threadpool(UserOptions options) {
             super("search-handler", options);
-            this.cluster = cluster;
         }
 
         @Override
-        public void getConfig(ContainerThreadpoolConfig.Builder builder) {
-            super.getConfig(builder);
-
-            builder.maxThreadExecutionTimeSeconds(190);
-            builder.keepAliveTime(5.0);
-
-            // User options overrides below configuration
-            if (hasUserOptions()) return;
-            builder.maxThreads(-2).minThreads(-2).queueSize(-40);
+        public void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder) {
+            builder.maxThreadExecutionTimeSeconds(190)
+                    .keepAliveTime(5.0)
+                    .maxThreads(-2)
+                    .minThreads(-2)
+                    .queueSize(-40);
         }
 
-
     }
+
 }