From 55c2f790177968aa9023709ae143c87bd3c1aec6 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 8 Jul 2022 01:31:54 +0200 Subject: Inject a default threadpool for all handlers that don't declare their own. --- .../vespa/model/clients/ContainerDocumentApi.java | 39 ++++++++++------------ .../vespa/model/container/ContainerCluster.java | 16 ++++++--- .../vespa/model/container/ContainerThreadpool.java | 10 +++--- .../vespa/model/container/component/Handler.java | 38 +++++++++++++++++++++ .../component/chain/ProcessingHandler.java | 12 +++---- .../vespa/model/container/xml/SearchHandler.java | 31 ++++++++--------- 6 files changed, 93 insertions(+), 53 deletions(-) (limited to 'config-model') 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> newVespaClientHandler( - String componentId, - String bindingSuffix, - HandlerOptions handlerOptions) { - Handler> handler = handlerComponentSpecification(componentId); + private static Handler> newVespaClientHandler(String componentId, + String bindingSuffix, + HandlerOptions handlerOptions, + Threadpool executor) { + Handler> 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> handlerComponentSpecification(String className) { - return new Handler<>(new ComponentModel( - BundleInstantiationSpecification.getFromStrings(className, null, "vespaclient-container-plugin"), "")); + private static Handler> 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 d24651e9a46..a3cc2adf1c9 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 @@ -144,6 +144,7 @@ public abstract class ContainerCluster private ContainerDocproc containerDocproc; private ContainerDocumentApi containerDocumentApi; private SecretStore secretStore; + private final ContainerThreadpool defaultHandlerThreadpool = new Handler.DefaultHandlerThreadpool(); private boolean rpcServerEnabled = true; private boolean httpServerEnabled = true; @@ -183,6 +184,7 @@ public abstract class ContainerCluster 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"); @@ -249,6 +251,15 @@ public abstract class ContainerCluster 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) { @@ -308,10 +319,7 @@ public abstract class ContainerCluster this.processingChains = processingChains; // Cannot use the class object for ProcessingHandler, because its superclass is not accessible - ProcessingHandler processingHandler = new ProcessingHandler<>( - processingChains, - PROCESSING_HANDLER_CLASS, - null); + ProcessingHandler processingHandler = new ProcessingHandler<>(processingChains, PROCESSING_HANDLER_CLASS); 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() { 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> extends Component< private final Set serverBindings = new LinkedHashSet<>(); private final List 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> fromClassName(String className) { @@ -53,4 +71,24 @@ public class Handler> 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 4754bf4c842..bc4a0eba89b 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; @@ -22,15 +24,11 @@ public class ProcessingHandler> 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 { 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 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); } - } + } -- cgit v1.2.3