diff options
author | Håkon Hallingstad <hakon.hallingstad@gmail.com> | 2022-07-11 20:44:01 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-07-11 20:44:01 +0200 |
commit | 7605979957b492e5cb9b7035a3a78c63524757fb (patch) | |
tree | 25a00ec20003c1df513bf430370c6f558f9618c5 | |
parent | daea91c18d4f000c5282ccff2daca7843fc12ab8 (diff) |
Revert "Inject default threadpool to handlers [run-systemtest]"
30 files changed, 219 insertions, 316 deletions
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 index c50b9b7f842..75b13a89e83 100644 --- 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 @@ -36,7 +36,7 @@ public class LogserverContainerCluster extends ContainerCluster<LogserverContain protected boolean messageBusEnabled() { return false; } private void addLogHandler() { - Handler logHandler = Handler.fromClassName(ContainerCluster.LOG_HANDLER_CLASS); + Handler<?> logHandler = Handler.fromClassName(ContainerCluster.LOG_HANDLER_CLASS); logHandler.addServerBindings(SystemBindingPattern.fromHttpPath("/logs")); addComponent(logHandler); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index 5d60cec0679..1da5e190c70 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -118,7 +118,7 @@ public class ClusterControllerContainer extends Container implements ZOOKEEPER_SERVER_BUNDLE); } - private void addHandler(Handler h, String path) { + private void addHandler(Handler<?> h, String path) { h.addServerBindings(SystemBindingPattern.fromHttpPath(path)); super.addHandler(h); } @@ -138,7 +138,7 @@ public class ClusterControllerContainer extends Container implements } private void addHandler(String id, String className, String path, ComponentSpecification bundle) { - addHandler(new Handler(createComponentModel(id, className, bundle)), path); + addHandler(new Handler<>(createComponentModel(id, className, bundle)), path); } private ReindexingContext reindexingContext() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index 680a4b97f86..a29647b062a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -119,12 +119,12 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC } private void addHttpHandler(Class<? extends ThreadedHttpRequestHandler> clazz, String bindingPath) { - Handler metricsHandler = createMetricsHandler(clazz, bindingPath); + Handler<AbstractConfigProducer<?>> metricsHandler = createMetricsHandler(clazz, bindingPath); addComponent(metricsHandler); } - static Handler createMetricsHandler(Class<? extends ThreadedHttpRequestHandler> clazz, String bindingPath) { - Handler metricsHandler = new Handler( + static Handler<AbstractConfigProducer<?>> createMetricsHandler(Class<? extends ThreadedHttpRequestHandler> clazz, String bindingPath) { + Handler<AbstractConfigProducer<?>> metricsHandler = new Handler<>( new ComponentModel(clazz.getName(), null, METRICS_PROXY_BUNDLE_NAME, null)); metricsHandler.addServerBindings( SystemBindingPattern.fromHttpPath(bindingPath), diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java index 718f1646126..4f322578b1c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java @@ -24,7 +24,7 @@ class UriBindingsValidator extends Validator { @Override public void validate(VespaModel model, DeployState deployState) { for (ApplicationContainerCluster cluster : model.getContainerClusters().values()) { - for (Handler handler : cluster.getHandlers()) { + for (Handler<?> handler : cluster.getHandlers()) { for (BindingPattern binding : handler.getServerBindings()) { validateUserBinding(binding, model, deployState); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomClientProviderBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomClientProviderBuilder.java index 170e8940787..69613944e74 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomClientProviderBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomClientProviderBuilder.java @@ -5,6 +5,7 @@ import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.text.XML; import com.yahoo.vespa.model.container.ApplicationContainerCluster; +import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.container.component.UserBindingPattern; import org.w3c.dom.Element; @@ -21,7 +22,7 @@ public class DomClientProviderBuilder extends DomHandlerBuilder { @Override protected Handler doBuild(DeployState deployState, AbstractConfigProducer parent, Element clientElement) { - Handler client = createHandler(clientElement); + Handler<? super Component<?, ?>> client = createHandler(clientElement); for (Element binding : XML.getChildren(clientElement, "binding")) client.addClientBindings(UserBindingPattern.fromPattern(XML.getValue(binding))); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java index 7bfe971981e..025a6377b04 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomHandlerBuilder.java @@ -9,12 +9,15 @@ import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.text.XML; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.component.BindingPattern; +import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.container.component.UserBindingPattern; import com.yahoo.vespa.model.container.xml.BundleInstantiationSpecificationBuilder; import org.w3c.dom.Element; +import java.util.List; import java.util.Set; +import java.util.logging.Level; import static com.yahoo.vespa.model.container.ApplicationContainerCluster.METRICS_V2_HANDLER_BINDING_1; import static com.yahoo.vespa.model.container.ApplicationContainerCluster.METRICS_V2_HANDLER_BINDING_2; @@ -26,7 +29,7 @@ import static java.util.logging.Level.INFO; /** * @author gjoranv */ -public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Handler> { +public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Handler<?>> { private static final Set<BindingPattern> reservedBindings = Set.of(METRICS_V2_HANDLER_BINDING_1, @@ -42,8 +45,8 @@ public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilder< } @Override - protected Handler doBuild(DeployState deployState, AbstractConfigProducer<?> parent, Element handlerElement) { - Handler handler = createHandler(handlerElement); + protected Handler<?> doBuild(DeployState deployState, AbstractConfigProducer<?> parent, Element handlerElement) { + Handler<? super Component<?, ?>> handler = createHandler(handlerElement); for (Element binding : XML.getChildren(handlerElement, "binding")) addServerBinding(handler, UserBindingPattern.fromPattern(XML.getValue(binding)), deployState.getDeployLogger()); @@ -53,18 +56,18 @@ public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilder< return handler; } - Handler createHandler(Element handlerElement) { + Handler<? super Component<?, ?>> createHandler(Element handlerElement) { BundleInstantiationSpecification bundleSpec = BundleInstantiationSpecificationBuilder.build(handlerElement); - return new Handler(new ComponentModel(bundleSpec)); + return new Handler<>(new ComponentModel(bundleSpec)); } - private void addServerBinding(Handler handler, BindingPattern binding, DeployLogger log) { + private void addServerBinding(Handler<? super Component<?, ?>> handler, BindingPattern binding, DeployLogger log) { throwIfBindingIsReserved(binding, handler); handler.addServerBindings(binding); removeExistingServerBinding(binding, handler, log); } - private void throwIfBindingIsReserved(BindingPattern binding, Handler newHandler) { + private void throwIfBindingIsReserved(BindingPattern binding, Handler<?> newHandler) { for (var reserved : reservedBindings) { if (binding.hasSamePattern(reserved)) { throw new IllegalArgumentException("Binding '" + binding.patternString() + "' is a reserved Vespa binding and " + @@ -73,7 +76,7 @@ public class DomHandlerBuilder extends VespaDomBuilder.DomConfigProducerBuilder< } } - private void removeExistingServerBinding(BindingPattern binding, Handler newHandler, DeployLogger log) { + private void removeExistingServerBinding(BindingPattern binding, Handler<?> newHandler, DeployLogger log) { for (var handler : cluster.getHandlers()) { for (BindingPattern serverBinding : handler.getServerBindings()) { if (serverBinding.hasSamePattern(binding)) { 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 d5f41353616..33c125dcecf 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 @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. 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; @@ -39,31 +41,31 @@ public class ContainerDocumentApi { private static void addFeedHandler(ContainerCluster<?> cluster, HandlerOptions handlerOptions) { String bindingSuffix = ContainerCluster.RESERVED_URI_PREFIX + "/feedapi"; - var executor = new Threadpool("feedapi-handler", handlerOptions.feedApiThreadpoolOptions); - var handler = newVespaClientHandler("com.yahoo.vespa.http.server.FeedHandler", - bindingSuffix, handlerOptions, executor); + 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); } private static void addRestApiHandler(ContainerCluster<?> cluster, HandlerOptions handlerOptions) { - var handler = newVespaClientHandler("com.yahoo.document.restapi.resource.DocumentV1ApiHandler", - DOCUMENT_V1_PREFIX + "/*", handlerOptions, null); + var handler = newVespaClientHandler("com.yahoo.document.restapi.resource.DocumentV1ApiHandler", DOCUMENT_V1_PREFIX + "/*", handlerOptions); 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 = createHandler("com.yahoo.document.restapi.resource.RestApi", null); + var oldHandlerDummy = handlerComponentSpecification("com.yahoo.document.restapi.resource.RestApi"); cluster.addComponent(oldHandlerDummy); } public boolean ignoreUndefinedFields() { return ignoreUndefinedFields; } - private static Handler newVespaClientHandler(String componentId, - String bindingSuffix, - HandlerOptions handlerOptions, - Threadpool executor) { - Handler handler = createHandler(componentId, executor); + private static Handler<AbstractConfigProducer<?>> newVespaClientHandler( + String componentId, + String bindingSuffix, + HandlerOptions handlerOptions) { + Handler<AbstractConfigProducer<?>> handler = handlerComponentSpecification(componentId); if (handlerOptions.bindings.isEmpty()) { handler.addServerBindings( SystemBindingPattern.fromHttpPath(bindingSuffix), @@ -79,9 +81,9 @@ public class ContainerDocumentApi { return handler; } - private static Handler createHandler(String className, Threadpool executor) { - return new Handler(new ComponentModel(className, null, "vespaclient-container-plugin"), - executor); + private static Handler<AbstractConfigProducer<?>> handlerComponentSpecification(String className) { + return new Handler<>(new ComponentModel( + BundleInstantiationSpecification.getFromStrings(className, null, "vespaclient-container-plugin"), "")); } public static final class HandlerOptions { @@ -102,10 +104,12 @@ public class ContainerDocumentApi { } @Override - protected void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder) { - builder.maxThreads(-4) - .minThreads(-4) - .queueSize(500); + public void getConfig(ContainerThreadpoolConfig.Builder builder) { + super.getConfig(builder); + + // User options overrides below configuration + if (hasUserOptions()) return; + builder.maxThreads(-4).minThreads(-4).queueSize(500); } } 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 index e316f826ad6..937d7cf58d3 100644 --- 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 @@ -147,7 +147,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat } private void addMetricsHandler(String handlerClass, BindingPattern rootBinding, BindingPattern innerBinding) { - Handler handler = new Handler( + Handler<AbstractConfigProducer<?>> handler = new Handler<>( new ComponentModel(handlerClass, null, null, null)); handler.addServerBindings(rootBinding, innerBinding); addComponent(handler); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java index 3d6332a9773..63c323bfdaf 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java @@ -76,7 +76,7 @@ public abstract class Container extends AbstractService implements private final boolean dumpHeapOnShutdownTimeout; private final double shutdownTimeoutS; - private final ComponentGroup<Handler> handlers = new ComponentGroup<>(this, "handler"); + private final ComponentGroup<Handler<?>> handlers = new ComponentGroup<>(this, "handler"); private final ComponentGroup<Component<?, ?>> components = new ComponentGroup<>(this, "components"); private final JettyHttpServer defaultHttpServer; @@ -113,7 +113,7 @@ public abstract class Container extends AbstractService implements /** True if this container is retired (slated for removal) */ public boolean isRetired() { return retired; } - public ComponentGroup<Handler> getHandlers() { + public ComponentGroup<Handler<?>> getHandlers() { return handlers; } @@ -129,7 +129,7 @@ public abstract class Container extends AbstractService implements addComponent(new SimpleComponent(new ComponentModel(idSpec, classSpec, bundleSpec))); } - public final void addHandler(Handler h) { + public final void addHandler(Handler<?> h) { handlers.addComponent(h); } 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 45fde57fcc7..2385b5b3812 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,7 +142,6 @@ 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; @@ -182,7 +181,6 @@ 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"); @@ -219,14 +217,14 @@ public abstract class ContainerCluster<CONTAINER extends Container> } public void addMetricStateHandler() { - Handler stateHandler = new Handler( + Handler<AbstractConfigProducer<?>> stateHandler = new Handler<>( new ComponentModel(STATE_HANDLER_CLASS, null, null, null)); stateHandler.addServerBindings(STATE_HANDLER_BINDING_1, STATE_HANDLER_BINDING_2); addComponent(stateHandler); } public void addDefaultRootHandler() { - Handler handler = new Handler( + Handler<AbstractConfigProducer<?>> handler = new Handler<>( new ComponentModel(BundleInstantiationSpecification.getFromStrings( BINDINGS_OVERVIEW_HANDLER_CLASS, null, null), null)); // null bundle, as the handler is in container-disc handler.addServerBindings(ROOT_HANDLER_BINDING); @@ -234,7 +232,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> } public void addApplicationStatusHandler() { - Handler statusHandler = new Handler( + Handler<AbstractConfigProducer<?>> statusHandler = new Handler<>( new ComponentModel(BundleInstantiationSpecification.getFromStrings( APPLICATION_STATUS_HANDLER_CLASS, null, null), null)); // null bundle, as the handler is in container-disc statusHandler.addServerBindings(SystemBindingPattern.fromHttpPath("/ApplicationStatus")); @@ -242,22 +240,13 @@ public abstract class ContainerCluster<CONTAINER extends Container> } public void addVipHandler() { - Handler vipHandler = Handler.fromClassName(FileStatusHandlerComponent.CLASS); + Handler<?> vipHandler = Handler.fromClassName(FileStatusHandlerComponent.CLASS); vipHandler.addServerBindings(VIP_HANDLER_BINDING); addComponent(vipHandler); } 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) { @@ -317,8 +306,10 @@ 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"); + ProcessingHandler<?> processingHandler = new ProcessingHandler<>( + processingChains, + "com.yahoo.processing.handler.ProcessingHandler", + null); for (BindingPattern binding: serverBindings) processingHandler.addServerBindings(binding); @@ -377,8 +368,9 @@ public abstract class ContainerCluster<CONTAINER extends Container> return containerDocproc.getChains(); } - public Collection<Handler> getHandlers() { - return componentGroup.getComponents(Handler.class); + @SuppressWarnings("unchecked") + public Collection<Handler<?>> getHandlers() { + return (Collection<Handler<?>>)(Collection)componentGroup.getComponents(Handler.class); } public void setSecretStore(SecretStore secretStore) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java index 088465f56b1..cb8e6ba85ff 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java @@ -63,8 +63,8 @@ public class ContainerModelEvaluation implements rankProfileList.getConfig(builder); } - public static Handler getHandler() { - Handler handler = new Handler(new ComponentModel(REST_HANDLER_NAME, null, EVALUATION_BUNDLE_NAME)); + public static Handler<?> getHandler() { + Handler<?> handler = new Handler<>(new ComponentModel(REST_HANDLER_NAME, null, EVALUATION_BUNDLE_NAME)); handler.addServerBindings( SystemBindingPattern.fromHttpPath(REST_BINDING_PATH), SystemBindingPattern.fromHttpPath(REST_BINDING_PATH + "/*")); 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 fbd7bc9fe56..489e4cc135a 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 abstract class ContainerThreadpool extends SimpleComponent implements ContainerThreadpoolConfig.Producer { +public class ContainerThreadpool extends SimpleComponent implements ContainerThreadpoolConfig.Producer { private final String name; private final UserOptions userOptions; @@ -32,13 +32,8 @@ public abstract class ContainerThreadpool extends SimpleComponent implements Con 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); @@ -47,6 +42,9 @@ public abstract class ContainerThreadpool extends SimpleComponent implements Con } } + 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/DiscBindingsConfigGenerator.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/DiscBindingsConfigGenerator.java index e4a5c2cd440..76124b14209 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/DiscBindingsConfigGenerator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/DiscBindingsConfigGenerator.java @@ -14,16 +14,16 @@ import static java.util.stream.Collectors.toList; */ public class DiscBindingsConfigGenerator { - public static Map<String, Handlers.Builder> generate(Collection<? extends Handler> handlers) { + public static Map<String, Handlers.Builder> generate(Collection<? extends Handler<?>> handlers) { Map<String, Handlers.Builder> handlerBuilders = new LinkedHashMap<>(); - for (Handler handler : handlers) { + for (Handler<?> handler : handlers) { handlerBuilders.putAll(generate(handler)); } return handlerBuilders; } - public static <T extends Handler> Map<String, Handlers.Builder> generate(T handler) { + public static <T extends Handler<?>> Map<String, Handlers.Builder> generate(T handler) { if (handler.getServerBindings().isEmpty() && handler.getClientBindings().isEmpty()) return Collections.emptyMap(); 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 9f2bfe9251b..8ffdccae896 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 @@ -1,9 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.container.component; -import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.osgi.provider.model.ComponentModel; -import com.yahoo.vespa.model.container.ContainerThreadpool; import java.util.ArrayList; import java.util.Arrays; @@ -16,35 +15,22 @@ import java.util.Set; * Models a jdisc RequestHandler (including ClientProvider). * RequestHandlers always have at least one server binding, * while ClientProviders have at least one client binding. + * <p> + * Note that this is also used to model vespa handlers (which do not have any bindings) * * @author gjoranv */ -public class Handler extends Component<Component<?, ?>, ComponentModel> { +public class Handler<CHILD extends AbstractConfigProducer<?>> extends Component<CHILD, ComponentModel> { 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); - } - - 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(threadpool); - inject(threadpool); - } else { - hasCustomThreadPool = false; - } } - public static Handler fromClassName(String className) { - return new Handler(new ComponentModel(className, null, null, null)); + public static Handler<AbstractConfigProducer<?>> fromClassName(String className) { + return new Handler<>(new ComponentModel(className, null, null, null)); } public void addServerBindings(BindingPattern... bindings) { @@ -67,24 +53,4 @@ public class Handler extends Component<Component<?, ?>, ComponentModel> { 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 e0bbc2d755d..3f68a0a2709 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 @@ -4,7 +4,7 @@ package com.yahoo.vespa.model.container.component.chain; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.core.ChainsConfig; import com.yahoo.osgi.provider.model.ComponentModel; -import com.yahoo.vespa.model.container.ContainerThreadpool; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.model.container.component.Handler; @@ -14,17 +14,21 @@ import com.yahoo.vespa.model.container.component.Handler; * @author gjoranv */ public class ProcessingHandler<CHAINS extends Chains<?>> - extends Handler + extends Handler<AbstractConfigProducer<?>> implements ChainsConfig.Producer { protected final CHAINS chains; public ProcessingHandler(CHAINS chains, String handlerClass) { - this(chains, BundleInstantiationSpecification.getInternalProcessingSpecificationFromStrings(handlerClass, null), null); + this(chains, BundleInstantiationSpecification.getInternalProcessingSpecificationFromStrings(handlerClass, null)); } - public ProcessingHandler(CHAINS chains, BundleInstantiationSpecification spec, ContainerThreadpool threadpool) { - super(new ComponentModel(spec, null), threadpool); + 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)); this.chains = chains; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/MbusClient.java b/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/MbusClient.java index 0efcd8df37f..28a0748be26 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/MbusClient.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/MbusClient.java @@ -6,12 +6,13 @@ import com.yahoo.component.ComponentSpecification; import com.yahoo.container.jdisc.config.SessionConfig; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.osgi.provider.model.ComponentModel; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.model.container.component.Handler; /** * @author Einar M R Rosenvinge */ -public class MbusClient extends Handler implements SessionConfig.Producer { +public class MbusClient extends Handler<AbstractConfigProducer<?>> implements SessionConfig.Producer { private static final ComponentSpecification CLASSNAME = ComponentSpecification.fromString("com.yahoo.container.jdisc.messagebus.MbusClientProvider"); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java index 3ee0414bf32..d85f00a5bb2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java @@ -51,7 +51,7 @@ public class AccessControl { private final String domain; private ClientAuthentication clientAuthentication = ClientAuthentication.need; private final Set<BindingPattern> excludeBindings = new LinkedHashSet<>(); - private Collection<Handler> handlers = Collections.emptyList(); + private Collection<Handler<?>> handlers = Collections.emptyList(); public Builder(String domain) { this.domain = domain; } @@ -79,12 +79,12 @@ public class AccessControl { public final String domain; public final ClientAuthentication clientAuthentication; private final Set<BindingPattern> excludedBindings; - private final Collection<Handler> handlers; + private final Collection<Handler<?>> handlers; private AccessControl(String domain, ClientAuthentication clientAuthentication, Set<BindingPattern> excludedBindings, - Collection<Handler> handlers) { + Collection<Handler<?>> handlers) { this.domain = domain; this.clientAuthentication = clientAuthentication; this.excludedBindings = Collections.unmodifiableSet(excludedBindings); @@ -119,7 +119,7 @@ public class AccessControl { public Set<BindingPattern> excludedBindings() { return excludedBindings; } /** all handlers (that are known by the access control components) **/ - public Collection<Handler> handlers() { return handlers; } + public Collection<Handler<?>> handlers() { return handlers; } public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) { return cluster.getHandlers().stream() @@ -135,7 +135,7 @@ public class AccessControl { for (BindingPattern excludedBinding : excludedBindings) { http.getBindings().add(createAccessControlExcludedBinding(excludedBinding)); } - for (Handler handler : handlers) { + for (Handler<?> handler : handlers) { if (isExcludedHandler(handler)) { for (BindingPattern binding : handler.getServerBindings()) { http.getBindings().add(createAccessControlExcludedBinding(binding)); @@ -188,9 +188,9 @@ public class AccessControl { private static Chain<Filter> createChain(ComponentId id) { return new Chain<>(FilterChains.emptyChainSpec(id)); } - private static boolean isExcludedHandler(Handler handler) { return EXCLUDED_HANDLERS.contains(handler.getClassId().getName()); } + private static boolean isExcludedHandler(Handler<?> handler) { return EXCLUDED_HANDLERS.contains(handler.getClassId().getName()); } - private static boolean hasNonMbusBinding(Handler handler) { + private static boolean hasNonMbusBinding(Handler<?> handler) { return handler.getServerBindings().stream().anyMatch(binding -> ! binding.scheme().equals("mbus")); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/GUIHandler.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/GUIHandler.java index c6b0eb28ed3..7087cabafc1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/GUIHandler.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/GUIHandler.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.search; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.vespa.model.container.component.Handler; @@ -9,7 +10,7 @@ import com.yahoo.vespa.model.container.component.Handler; /** * @author Henrik Høiness */ -public class GUIHandler extends Handler { +public class GUIHandler extends Handler<AbstractConfigProducer<?>> { public static final String BUNDLE = "container-search-gui"; public static final String CLASS = "com.yahoo.search.query.gui.GUIHandler"; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 956d7e5c366..1c47f1d7c9c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -222,7 +222,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } if (deployState.zone().system().isPublic()) { BindingPattern bindingPattern = SystemBindingPattern.fromHttpPath("/validate-secret-store"); - Handler handler = new Handler( + Handler<AbstractConfigProducer<?>> handler = new Handler<>( new ComponentModel("com.yahoo.jdisc.cloud.aws.AwsParameterStoreValidationHandler", null, "jdisc-cloud-aws", null)); handler.addServerBindings(bindingPattern); cluster.addComponent(handler); @@ -897,7 +897,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } private void addGUIHandler(ApplicationContainerCluster cluster) { - Handler guiHandler = new GUIHandler(); + Handler<?> guiHandler = new GUIHandler(); guiHandler.addServerBindings(SystemBindingPattern.fromHttpPath(GUIHandler.BINDING_PATH)); cluster.addComponent(guiHandler); } 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 6b0bf8a67b9..54d943f498a 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,7 +1,6 @@ // 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; @@ -12,8 +11,6 @@ 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} * @@ -22,32 +19,38 @@ import static com.yahoo.container.bundle.BundleInstantiationSpecification.getInt 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_SPEC, new Threadpool(threadpoolOptions)); + super(cluster.getSearchChains(), HANDLER_CLASS); 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(UserOptions options) { + Threadpool(ApplicationContainerCluster cluster, UserOptions options) { super("search-handler", options); + this.cluster = cluster; } @Override - public void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder) { - builder.maxThreadExecutionTimeSeconds(190) - .keepAliveTime(5.0) - .maxThreads(-2) - .minThreads(-2) - .queueSize(-40); + 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); } - } + } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index d50aedeafa0..7bf08461df7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -70,7 +70,7 @@ public class MetricsProxyContainerClusterTest { @Test public void http_handlers_are_set_up() { VespaModel model = getModel(servicesWithAdminOnly(), self_hosted); - Collection<Handler> handlers = model.getAdmin().getMetricsProxyCluster().getHandlers(); + Collection<Handler<?>> handlers = model.getAdmin().getMetricsProxyCluster().getHandlers(); Collection<ComponentSpecification> handlerClasses = handlers.stream().map(Component::getClassId).collect(toList()); assertTrue(handlerClasses.contains(ComponentSpecification.fromString(MetricsV1Handler.class.getName()))); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java index d9659cf92d4..81b6de8f2ee 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java @@ -89,7 +89,7 @@ public class AccessLogTest extends ContainerModelBuilderTestBase { assertNotNull(getVespaAccessLog("default")); { // vespa - Component<?, ?> accessLogComponent = getComponent("default", VespaAccessLog.class.getName()); + Component<?, ?> accessLogComponent = getContainerComponent("default", VespaAccessLog.class.getName()); assertNotNull(accessLogComponent); assertEquals(VespaAccessLog.class.getName(), accessLogComponent.getClassId().getName(), VespaAccessLog.class.getName()); AccessLogConfig config = root.getConfig(AccessLogConfig.class, "default/component/com.yahoo.container.logging.VespaAccessLog"); @@ -101,7 +101,7 @@ public class AccessLogTest extends ContainerModelBuilderTestBase { } { // json - Component<?, ?> accessLogComponent = getComponent("default", JSONAccessLog.class.getName()); + Component<?, ?> accessLogComponent = getContainerComponent("default", JSONAccessLog.class.getName()); assertNotNull(accessLogComponent); assertEquals(JSONAccessLog.class.getName(), accessLogComponent.getClassId().getName(), JSONAccessLog.class.getName()); AccessLogConfig config = root.getConfig(AccessLogConfig.class, "default/component/com.yahoo.container.logging.JSONAccessLog"); @@ -124,7 +124,7 @@ public class AccessLogTest extends ContainerModelBuilderTestBase { nodesXml, "</container>" ); createModel(root, clusterElem); - Component<?, ?> connectionLogComponent = getComponent("default", FileConnectionLog.class.getName()); + Component<?, ?> connectionLogComponent = getContainerComponent("default", FileConnectionLog.class.getName()); assertNotNull(connectionLogComponent); ConnectionLogConfig config = root.getConfig(ConnectionLogConfig.class, "default/component/com.yahoo.container.logging.FileConnectionLog"); assertEquals("default", config.cluster()); @@ -140,7 +140,7 @@ public class AccessLogTest extends ContainerModelBuilderTestBase { nodesXml, "</container>" ); createModel(root, clusterElem); - Component<?, ?> fileConnectionLogComponent = getComponent("default", FileConnectionLog.class.getName()); + Component<?, ?> fileConnectionLogComponent = getContainerComponent("default", FileConnectionLog.class.getName()); assertNull(fileConnectionLogComponent); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ApplicationBuilderTest.java index c41373b9f85..ca59e053a89 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ApplicationBuilderTest.java @@ -31,7 +31,7 @@ import static org.junit.Assert.fail; /** * @author gjoranv */ -public class SearchBuilderTest extends ContainerModelBuilderTestBase { +public class ApplicationBuilderTest extends ContainerModelBuilderTestBase { private ChainsConfig chainsConfig() { return root.getConfig(ChainsConfig.class, "default/component/com.yahoo.search.handler.SearchHandler"); @@ -53,7 +53,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { ApplicationContainerCluster cluster = (ApplicationContainerCluster)root.getChildren().get("default"); GUIHandler guiHandler = null; - for (Handler handler : cluster.getHandlers()) { + for (Handler<?> handler : cluster.getHandlers()) { if (handler instanceof GUIHandler) { guiHandler = (GUIHandler) handler; } @@ -230,7 +230,12 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { createModel(root, clusterElem); - Handler searchHandler = getHandler("default", SearchHandler.HANDLER_CLASS); + ApplicationContainerCluster cluster = (ApplicationContainerCluster)root.getChildren().get("default"); + Handler<?> searchHandler = cluster.getHandlers().stream() + .filter(h -> h.getComponentId().toString().equals(SearchHandler.HANDLER_CLASS)) + .findAny() + .get(); + assertTrue(searchHandler.getInjectedComponentIds().contains("threadpool@search-handler")); ContainerThreadpoolConfig config = root.getConfig( diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java index 3cab01fd038..ca0b4681e51 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java @@ -30,11 +30,11 @@ import static org.junit.Assert.assertTrue; */ public class ContainerDocumentApiBuilderTest extends ContainerModelBuilderTestBase { - private Map<String, Handler> getHandlers(String clusterName) { + private Map<String, Handler<?>> getHandlers(String clusterName) { ContainerCluster<?> cluster = (ContainerCluster<?>) root.getChildren().get(clusterName); - Map<String, Handler> handlerMap = new HashMap<>(); - Collection<Handler> handlers = cluster.getHandlers(); - for (Handler handler : handlers) { + Map<String, Handler<?>> handlerMap = new HashMap<>(); + Collection<Handler<?>> handlers = cluster.getHandlers(); + for (Handler<?> handler : handlers) { assertFalse(handlerMap.containsKey(handler.getComponentId().toString())); //die on overwrites handlerMap.put(handler.getComponentId().toString(), handler); } @@ -56,7 +56,7 @@ public class ContainerDocumentApiBuilderTest extends ContainerModelBuilderTestBa } private void verifyCustomBindings(String id) { - Handler handler = getHandlers("cluster1").get(id); + Handler<?> handler = getHandlers("cluster1").get(id); assertTrue(handler.getServerBindings().contains(UserBindingPattern.fromHttpPath("/document-api/reserved-for-internal-use/feedapi"))); assertTrue(handler.getServerBindings().contains(UserBindingPattern.fromHttpPath("/document-api/reserved-for-internal-use/feedapi/"))); @@ -73,7 +73,7 @@ public class ContainerDocumentApiBuilderTest extends ContainerModelBuilderTestBa "</container>"); createModel(root, elem); - Map<String, Handler> handlerMap = getHandlers("cluster1"); + Map<String, Handler<?>> handlerMap = getHandlers("cluster1"); assertNotNull(handlerMap.get("com.yahoo.container.handler.VipStatusHandler")); assertNotNull(handlerMap.get("com.yahoo.container.handler.observability.ApplicationStatusHandler")); @@ -110,8 +110,8 @@ public class ContainerDocumentApiBuilderTest extends ContainerModelBuilderTestBa "</container>"); root = new MockRoot("root", new MockApplicationPackage.Builder().build()); createModel(root, elem); - Map<String, Handler> handlers = getHandlers("cluster1"); - Handler feedApiHandler = handlers.get("com.yahoo.vespa.http.server.FeedHandler"); + Map<String, Handler<?>> handlers = getHandlers("cluster1"); + Handler<?> feedApiHandler = handlers.get("com.yahoo.vespa.http.server.FeedHandler"); Set<String> injectedComponentIds = feedApiHandler.getInjectedComponentIds(); assertTrue(injectedComponentIds.contains("threadpool@feedapi-handler")); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 2ab81a7a4bb..3a241d9607f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -21,6 +21,7 @@ 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.container.QrConfig; import com.yahoo.container.core.ChainsConfig; import com.yahoo.container.core.VipStatusConfig; @@ -41,7 +42,6 @@ import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.ContainerModelEvaluation; import com.yahoo.vespa.model.container.component.Component; -import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.content.utils.ContentClusterUtils; import com.yahoo.vespa.model.test.VespaModelTester; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; @@ -60,14 +60,19 @@ import java.util.stream.Collectors; import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.test.LinePatternMatcher.containsLineWithPattern; import static com.yahoo.vespa.defaults.Defaults.getDefaults; +import static com.yahoo.vespa.model.container.ContainerCluster.ROOT_HANDLER_BINDING; +import static com.yahoo.vespa.model.container.ContainerCluster.STATE_HANDLER_BINDING_1; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -193,17 +198,6 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test - public void builtin_handlers_get_default_threadpool() { - createBasicContainerModel(); - - Handler h1 = getHandler("default", ApplicationStatusHandler.class.getName()); - assertTrue(h1.getInjectedComponentIds().contains("threadpool@default-handler-common")); - - Handler h2 = getHandler("default", BindingsOverviewHandler.class.getName()); - assertTrue(h2.getInjectedComponentIds().contains("threadpool@default-handler-common")); - } - - @Test public void verify_bindings_for_builtin_handlers() { createBasicContainerModel(); JdiscBindingsConfig config = root.getConfig(JdiscBindingsConfig.class, "default/container.0"); @@ -222,6 +216,68 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test + public void default_root_handler_binding_can_be_stolen_by_user_configured_handler() { + Element clusterElem = DomBuilderTest.parse( + "<container id='default' version='1.0'>" + + " <handler id='userRootHandler'>" + + " <binding>" + ROOT_HANDLER_BINDING.patternString() + "</binding>" + + " </handler>" + + "</container>"); + createModel(root, clusterElem); + + // The handler is still set up. + ComponentsConfig.Components userRootHandler = getComponent(componentsConfig(), BindingsOverviewHandler.class.getName()); + assertNotNull(userRootHandler); + + // .. but it has no bindings + var discBindingsConfig = root.getConfig(JdiscBindingsConfig.class, "default"); + assertNull(discBindingsConfig.handlers(BindingsOverviewHandler.class.getName())); + } + + @Test + public void reserved_binding_cannot_be_stolen_by_user_configured_handler() { + Element clusterElem = DomBuilderTest.parse( + "<container id='default' version='1.0'>" + + " <handler id='userHandler'>" + + " <binding>" + STATE_HANDLER_BINDING_1.patternString() + "</binding>" + + " </handler>" + + "</container>"); + try { + createModel(root, clusterElem); + fail("Expected exception when stealing a reserved binding."); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), is("Binding 'http://*/state/v1' is a reserved Vespa binding " + + "and cannot be used by handler: userHandler")); + } + } + + @Test + public void handler_bindings_are_included_in_discBindings_config() { + createClusterWithJDiscHandler(); + String discBindingsConfig = root.getConfig(JdiscBindingsConfig.class, "default").toString(); + assertThat(discBindingsConfig, containsString(".serverBindings[0] \"http://*/binding0\"")); + assertThat(discBindingsConfig, containsString(".serverBindings[1] \"http://*/binding1\"")); + } + + @Test + public void handlers_are_included_in_components_config() { + createClusterWithJDiscHandler(); + assertThat(componentsConfig().toString(), containsString(".id \"discHandler\"")); + } + + private void createClusterWithJDiscHandler() { + Element clusterElem = DomBuilderTest.parse( + "<container id='default' version='1.0'>", + " <handler id='discHandler'>", + " <binding>http://*/binding0</binding>", + " <binding>http://*/binding1</binding>", + " </handler>", + "</container>"); + + createModel(root, clusterElem); + } + + @Test public void processing_handler_bindings_can_be_overridden() { Element clusterElem = DomBuilderTest.parse( "<container id='default' version='1.0'>", @@ -319,6 +375,20 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test + public void nested_components_are_injected_to_handlers() { + Element clusterElem = DomBuilderTest.parse( + "<container id='default' version='1.0'>", + " <handler id='myHandler'>", + " <component id='injected' />", + " </handler>", + "</container>"); + + createModel(root, clusterElem); + Component<?,?> handler = getContainerComponent("default", "myHandler"); + assertThat(handler.getInjectedComponentIds(), hasItem("injected@myHandler")); + } + + @Test public void component_includes_are_added() { VespaModelCreatorWithFilePkg creator = new VespaModelCreatorWithFilePkg("src/test/cfg/application/include_dirs"); VespaModel model = creator.create(true); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java index dc831ef863d..3d7b17d37e0 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java @@ -13,7 +13,6 @@ import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.ContainerModel; import com.yahoo.vespa.model.container.component.Component; -import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.container.search.ContainerSearch; import org.junit.Before; import org.w3c.dom.Element; @@ -95,7 +94,7 @@ public abstract class ContainerModelBuilderTestBase { return root.getConfig(ComponentsConfig.class, "default"); } - protected ComponentsConfig.Components getComponentInConfig(ComponentsConfig componentsConfig, String id) { + protected ComponentsConfig.Components getComponent(ComponentsConfig componentsConfig, String id) { for (ComponentsConfig.Components component : componentsConfig.components()) { if (component.id().equals(id)) return component; @@ -107,18 +106,11 @@ public abstract class ContainerModelBuilderTestBase { return (ApplicationContainerCluster) root.getChildren().get(clusterId); } - public Component<?, ?> getComponent(String clusterId, String componentId) { + public Component<?, ?> getContainerComponent(String clusterId, String componentId) { return getContainerCluster(clusterId).getComponentsMap().get( ComponentId.fromString(componentId)); } - public Handler getHandler(String clusterId, String componentId) { - Component<?,?> component = getComponent(clusterId, componentId); - if (! (component instanceof Handler)) - throw new RuntimeException("Component is not a handler: " + componentId); - return (Handler) component; - } - void assertComponentConfigured(ApplicationContainerCluster cluster, String componentId) { Component<?, ?> component = cluster.getComponentsMap().get(ComponentId.fromString(componentId)); assertNotNull(component); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java deleted file mode 100644 index 42cda0d8034..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/HandlerBuilderTest.java +++ /dev/null @@ -1,118 +0,0 @@ -package com.yahoo.vespa.model.container.xml; - -import com.yahoo.config.model.builder.xml.test.DomBuilderTest; -import com.yahoo.container.ComponentsConfig; -import com.yahoo.container.jdisc.JdiscBindingsConfig; -import com.yahoo.container.usability.BindingsOverviewHandler; -import com.yahoo.vespa.model.container.ApplicationContainerCluster; -import com.yahoo.vespa.model.container.component.Component; -import com.yahoo.vespa.model.container.component.Handler; -import org.junit.Test; -import org.w3c.dom.Element; - -import static com.yahoo.vespa.model.container.ContainerCluster.ROOT_HANDLER_BINDING; -import static com.yahoo.vespa.model.container.ContainerCluster.STATE_HANDLER_BINDING_1; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItem; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -/** - * Tests for container model building with custom handlers. - * - * @author gjoranv - */ -public class HandlerBuilderTest extends ContainerModelBuilderTestBase { - - @Test - public void handlers_are_included_in_components_config() { - createClusterWithJDiscHandler(); - assertThat(componentsConfig().toString(), containsString(".id \"discHandler\"")); - } - - @Test - public void handler_bindings_are_included_in_discBindings_config() { - createClusterWithJDiscHandler(); - String discBindingsConfig = root.getConfig(JdiscBindingsConfig.class, "default").toString(); - assertThat(discBindingsConfig, containsString(".serverBindings[0] \"http://*/binding0\"")); - assertThat(discBindingsConfig, containsString(".serverBindings[1] \"http://*/binding1\"")); - } - - @Test - public void nested_components_are_injected_to_handlers() { - Element clusterElem = DomBuilderTest.parse( - "<container id='default' version='1.0'>", - " <handler id='myHandler'>", - " <component id='injected' />", - " </handler>", - "</container>"); - - createModel(root, clusterElem); - Component<?,?> handler = getComponent("default", "myHandler"); - assertThat(handler.getInjectedComponentIds(), hasItem("injected@myHandler")); - } - - @Test - public void default_root_handler_binding_can_be_stolen_by_user_configured_handler() { - Element clusterElem = DomBuilderTest.parse( - "<container id='default' version='1.0'>" + - " <handler id='userRootHandler'>" + - " <binding>" + ROOT_HANDLER_BINDING.patternString() + "</binding>" + - " </handler>" + - "</container>"); - createModel(root, clusterElem); - - // The handler is still set up. - ComponentsConfig.Components userRootHandler = getComponentInConfig(componentsConfig(), BindingsOverviewHandler.class.getName()); - assertNotNull(userRootHandler); - - // .. but it has no bindings - var discBindingsConfig = root.getConfig(JdiscBindingsConfig.class, "default"); - assertNull(discBindingsConfig.handlers(BindingsOverviewHandler.class.getName())); - } - - @Test - public void reserved_binding_cannot_be_stolen_by_user_configured_handler() { - Element clusterElem = DomBuilderTest.parse( - "<container id='default' version='1.0'>" + - " <handler id='userHandler'>" + - " <binding>" + STATE_HANDLER_BINDING_1.patternString() + "</binding>" + - " </handler>" + - "</container>"); - try { - createModel(root, clusterElem); - fail("Expected exception when stealing a reserved binding."); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is("Binding 'http://*/state/v1' is a reserved Vespa binding " + - "and cannot be used by handler: userHandler")); - } - } - - @Test - public void custom_handler_gets_default_threadpool() { - createClusterWithJDiscHandler(); - ApplicationContainerCluster cluster = (ApplicationContainerCluster)root.getChildren().get("default"); - Handler handler = cluster.getHandlers().stream() - .filter(h -> h.getComponentId().toString().equals("discHandler")) - .findAny().orElseThrow(); - - assertTrue(handler.getInjectedComponentIds().contains("threadpool@default-handler-common")); - } - - private void createClusterWithJDiscHandler() { - Element clusterElem = DomBuilderTest.parse( - "<container id='default' version='1.0'>", - " <handler id='discHandler'>", - " <binding>http://*/binding0</binding>", - " <binding>http://*/binding1</binding>", - " </handler>", - "</container>"); - - createModel(root, clusterElem); - } - -} diff --git a/config-model/src/test/schema-test-files/services.xml b/config-model/src/test/schema-test-files/services.xml index df87d2b9f3d..4fb81963152 100644 --- a/config-model/src/test/schema-test-files/services.xml +++ b/config-model/src/test/schema-test-files/services.xml @@ -193,9 +193,6 @@ <handler id="jdisc-handler"> <binding>http://*:*/HelloWorld</binding> <binding>http://*:*/Status</binding> - <component id="injected-to-handler"> - <config name="foo"/> - </component> </handler> <server id="server-provider"> diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java index fb7ba1ab39e..11355dee24f 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java @@ -50,7 +50,7 @@ public abstract class ThreadedHttpRequestHandler extends ThreadedRequestHandler this(executor, metric, false); } - // TODO: deprecate this and the Context class. The context component set up in the model does not get a dedicated thread pool. + // TODO: move Inject annotation here! public ThreadedHttpRequestHandler(Context context) { this(context.executor, context.metric); } diff --git a/container-core/src/test/java/com/yahoo/container/di/componentgraph/core/ComponentGraphTest.java b/container-core/src/test/java/com/yahoo/container/di/componentgraph/core/ComponentGraphTest.java index 5abb96527bd..f8f567e1890 100644 --- a/container-core/src/test/java/com/yahoo/container/di/componentgraph/core/ComponentGraphTest.java +++ b/container-core/src/test/java/com/yahoo/container/di/componentgraph/core/ComponentGraphTest.java @@ -102,7 +102,7 @@ public class ComponentGraphTest { } @Test - public void component_can_be_explicitly_injected_into_another_component() { + public void component_can_be_injected_into_another_component() { Node injectedComponent = mockComponentNode(SimpleComponent.class); Node targetComponent = mockComponentNode(ComponentTakingComponent.class); targetComponent.inject(injectedComponent); @@ -117,22 +117,6 @@ public class ComponentGraphTest { ComponentTakingComponent instance = componentGraph.getInstance(ComponentTakingComponent.class); assertNotNull(instance); - assertSame(injectedComponent.instance.get(), instance.injectedComponent); - } - - @Test - public void explicitly_injected_components_may_be_unused() { - Node notUsingInjected = mockComponentNode(SimpleComponent.class); - Node injectedComponent = mockComponentNode(SimpleComponent2.class); - notUsingInjected.inject(injectedComponent); - - ComponentGraph componentGraph = new ComponentGraph(); - componentGraph.add(injectedComponent); - componentGraph.add(notUsingInjected); - componentGraph.complete(); - - SimpleComponent instanceNotUsingInjected = componentGraph.getInstance(SimpleComponent.class); - assertNotNull(instanceNotUsingInjected); } @Test @@ -530,7 +514,7 @@ public class ComponentGraphTest { } public static class ComponentTakingComponent extends AbstractComponent { - final SimpleComponent injectedComponent; + private final SimpleComponent injectedComponent; public ComponentTakingComponent(SimpleComponent injectedComponent) { assertNotNull(injectedComponent); |