diff options
author | Jon Bratseth <bratseth@oath.com> | 2019-06-10 12:25:21 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-10 12:25:21 +0200 |
commit | 32805405a1fcb7259c228c74dd88e0f64c3c9133 (patch) | |
tree | 5ed3403cb6350177ab59243cfb13948a6bd493d2 | |
parent | 79cd883d5df45dc236e5cebf2c21b5487c791df6 (diff) | |
parent | ef539860356ebcfe316ac8adc75aa183ab764ab4 (diff) |
Merge pull request #9740 from vespa-engine/bratseth/execution-factory
Extract execution creation into ExecutionFactory
23 files changed, 202 insertions, 116 deletions
diff --git a/application/src/main/java/com/yahoo/application/Application.java b/application/src/main/java/com/yahoo/application/Application.java index 64ced48edd3..f7b5174b0e5 100644 --- a/application/src/main/java/com/yahoo/application/Application.java +++ b/application/src/main/java/com/yahoo/application/Application.java @@ -46,7 +46,7 @@ public final class Application implements AutoCloseable { /** * This system property is set to "true" upon creation of an Application. - * This is useful for components which are created by dependendy injection which needs to modify + * This is useful for components which are created by dependendcy injection which needs to modify * their behavior to function without reliance on any processes outside the JVM. */ public static final String vespaLocalProperty = "vespa.local"; @@ -142,6 +142,7 @@ public final class Application implements AutoCloseable { */ @Beta public static class Builder { + private static final ThreadLocal<Random> random = new ThreadLocal<>(); private static final String DEFAULT_CHAIN = "default"; @@ -217,7 +218,7 @@ public final class Application implements AutoCloseable { } // copy from com.yahoo.application.ApplicationBuilder - private void createFile(final Path path, final String content) throws IOException { + private void createFile(Path path, String content) throws IOException { Files.createDirectories(path.getParent()); Files.write(path, Utf8.toBytes(content)); } @@ -237,16 +238,15 @@ public final class Application implements AutoCloseable { /** * @param name name of document type (search definition) * @param searchDefinition add this search definition to the application - * @return builder * @throws java.io.IOException e.g.if file not found */ - public Builder documentType(final String name, final String searchDefinition) throws IOException { + public Builder documentType(String name, String searchDefinition) throws IOException { Path path = nestedResource(ApplicationPackage.SEARCH_DEFINITIONS_DIR, name, ApplicationPackage.SD_NAME_SUFFIX); createFile(path, searchDefinition); return this; } - public Builder expressionInclude(final String name, final String searchDefinition) throws IOException { + public Builder expressionInclude(String name, String searchDefinition) throws IOException { Path path = nestedResource(ApplicationPackage.SEARCH_DEFINITIONS_DIR, name, ApplicationPackage.RANKEXPRESSION_NAME_SUFFIX); createFile(path, searchDefinition); return this; @@ -255,10 +255,9 @@ public final class Application implements AutoCloseable { /** * @param name name of rank expression * @param rankExpressionContent add this rank expression to the application - * @return builder * @throws java.io.IOException e.g.if file not found */ - public Builder rankExpression(final String name, final String rankExpressionContent) throws IOException { + public Builder rankExpression(String name, String rankExpressionContent) throws IOException { Path path = nestedResource(ApplicationPackage.SEARCH_DEFINITIONS_DIR, name, ApplicationPackage.RANKEXPRESSION_NAME_SUFFIX); createFile(path, rankExpressionContent); return this; @@ -270,7 +269,7 @@ public final class Application implements AutoCloseable { * @return builder * @throws java.io.IOException e.g.if file not found */ - public Builder queryProfile(final String name, final String queryProfile) throws IOException { + public Builder queryProfile(String name, String queryProfile) throws IOException { Path path = nestedResource(ApplicationPackage.QUERY_PROFILES_DIR, name, ".xml"); createFile(path, queryProfile); return this; @@ -282,14 +281,14 @@ public final class Application implements AutoCloseable { * @return builder * @throws java.io.IOException e.g.if file not found */ - public Builder queryProfileType(final String name, final String queryProfileType) throws IOException { + public Builder queryProfileType(String name, String queryProfileType) throws IOException { Path path = nestedResource(ApplicationPackage.QUERY_PROFILE_TYPES_DIR, name, ".xml"); createFile(path, queryProfileType); return this; } // copy from com.yahoo.application.ApplicationBuilder - private Path nestedResource(final com.yahoo.path.Path nestedPath, final String name, final String fileType) { + private Path nestedResource(com.yahoo.path.Path nestedPath, String name, String fileType) { String nameWithoutSuffix = StringUtilities.stripSuffix(name, fileType); return path.resolve(nestedPath.getRelative()).resolve(nameWithoutSuffix + fileType); } @@ -298,7 +297,7 @@ public final class Application implements AutoCloseable { * @param networking enable or disable networking (disabled by default) * @return builder */ - public Builder networking(final Networking networking) { + public Builder networking(Networking networking) { this.networking = networking; return this; } @@ -335,15 +334,14 @@ public final class Application implements AutoCloseable { private void generateXml() throws Exception { try (PrintWriter xml = new PrintWriter(Files.newOutputStream(path.resolve("services.xml")))) { xml.println("<?xml version=\"1.0\" encoding=\"utf-8\" ?>"); - //xml.println("<services version=\"1.0\">"); for (Map.Entry<String, Container> entry : containers.entrySet()) { entry.getValue().build(xml, entry.getKey(), (networking == Networking.enable ? getRandomPort() : -1)); } - //xml.println("</services>"); } } public static class Container { + private final Map<String, List<ComponentItem<? extends DocumentProcessor>>> docprocs = new LinkedHashMap<>(); private final Map<String, List<ComponentItem<? extends Searcher>>> searchers = new LinkedHashMap<>(); private final List<ComponentItem<? extends Renderer>> renderers = new ArrayList<>(); @@ -372,7 +370,7 @@ public final class Application implements AutoCloseable { * @param docproc add this docproc to the default document processing chain * @return builder */ - public Container documentProcessor(final Class<? extends DocumentProcessor> docproc) { + public Container documentProcessor(Class<? extends DocumentProcessor> docproc) { return documentProcessor(DEFAULT_CHAIN, docproc); } @@ -382,7 +380,7 @@ public final class Application implements AutoCloseable { * @param configs local docproc configs * @return builder */ - public Container documentProcessor(final String chainName, final Class<? extends DocumentProcessor> docproc, ConfigInstance... configs) { + public Container documentProcessor(String chainName, Class<? extends DocumentProcessor> docproc, ConfigInstance... configs) { return documentProcessor(docproc.getName(), chainName, docproc, configs); } @@ -393,7 +391,7 @@ public final class Application implements AutoCloseable { * @param configs local docproc configs * @return builder */ - public Container documentProcessor(String id, final String chainName, final Class<? extends DocumentProcessor> docproc, ConfigInstance... configs) { + public Container documentProcessor(String id, String chainName, Class<? extends DocumentProcessor> docproc, ConfigInstance... configs) { List<ComponentItem<? extends DocumentProcessor>> chain = docprocs.get(chainName); if (chain == null) { chain = new ArrayList<>(); @@ -416,7 +414,7 @@ public final class Application implements AutoCloseable { * @param searcher add this searcher to the default search chain * @return builder */ - public Container searcher(final Class<? extends Searcher> searcher) { + public Container searcher(Class<? extends Searcher> searcher) { return searcher(DEFAULT_CHAIN, searcher); } @@ -426,7 +424,7 @@ public final class Application implements AutoCloseable { * @param configs local searcher configs * @return builder */ - public Container searcher(final String chainName, final Class<? extends Searcher> searcher, ConfigInstance... configs) { + public Container searcher(String chainName, Class<? extends Searcher> searcher, ConfigInstance... configs) { return searcher(searcher.getName(), chainName, searcher, configs); } @@ -437,7 +435,7 @@ public final class Application implements AutoCloseable { * @param configs local searcher configs * @return builder */ - public Container searcher(String id, final String chainName, final Class<? extends Searcher> searcher, ConfigInstance... configs) { + public Container searcher(String id, String chainName, Class<? extends Searcher> searcher, ConfigInstance... configs) { List<ComponentItem<? extends Searcher>> chain = searchers.get(chainName); if (chain == null) { chain = new ArrayList<>(); @@ -453,7 +451,7 @@ public final class Application implements AutoCloseable { * @param configs local renderer configs * @return builder */ - public Container renderer(String id, final Class<? extends Renderer> renderer, ConfigInstance... configs) { + public Container renderer(String id, Class<? extends Renderer> renderer, ConfigInstance... configs) { renderers.add(new ComponentItem<>(id, renderer, configs)); return this; } @@ -463,7 +461,7 @@ public final class Application implements AutoCloseable { * @param handler the handler class * @return builder */ - public Container handler(final String binding, final Class<? extends RequestHandler> handler) { + public Container handler(String binding, Class<? extends RequestHandler> handler) { handlers.add(new ComponentItem<>(binding, handler)); return this; } @@ -473,7 +471,7 @@ public final class Application implements AutoCloseable { * @param client the client class * @return builder */ - public Container client(final String binding, final Class<? extends ClientProvider> client) { + public Container client(String binding, Class<? extends ClientProvider> client) { clients.add(new ComponentItem<>(binding, client)); return this; } @@ -483,7 +481,7 @@ public final class Application implements AutoCloseable { * @param server the server class * @return builder */ - public Container server(final String id, final Class<? extends ServerProvider> server) { + public Container server(String id, Class<? extends ServerProvider> server) { servers.add(new ComponentItem<>(id, server)); return this; } @@ -492,7 +490,7 @@ public final class Application implements AutoCloseable { * @param component make this component available to the container * @return builder */ - public Container component(final Class<?> component) { + public Container component(Class<?> component) { return component(component.getName(), component, (ConfigInstance) null); } @@ -500,7 +498,7 @@ public final class Application implements AutoCloseable { * @param component make this component available to the container * @return builder */ - public Container component(String id, final Class<?> component, ConfigInstance... configs) { + public Container component(String id, Class<?> component, ConfigInstance... configs) { components.add(new ComponentItem<>(id, component, configs)); return this; } diff --git a/application/src/main/java/com/yahoo/application/container/JDisc.java b/application/src/main/java/com/yahoo/application/container/JDisc.java index 45c7b66ac5e..10ff84d3ede 100644 --- a/application/src/main/java/com/yahoo/application/container/JDisc.java +++ b/application/src/main/java/com/yahoo/application/container/JDisc.java @@ -21,6 +21,7 @@ import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.test.TestDriver; import com.yahoo.processing.handler.ProcessingHandler; import com.yahoo.search.handler.SearchHandler; +import com.yahoo.search.searchchain.ExecutionFactory; import java.nio.file.Path; @@ -41,7 +42,7 @@ public final class JDisc implements AutoCloseable { @SuppressWarnings("unused") private final StandaloneContainerApplication application; - private final Container container = Container.get(); // TODO: This is indeed temporary ... *3 years later* Indeed. + private final Container container = Container.get(); // TODO: This is indeed temporary ... *3 years later* indeed. private final Path path; private final boolean deletePathWhenClosing; @@ -55,7 +56,7 @@ public final class JDisc implements AutoCloseable { application = (StandaloneContainerApplication) testDriver.application(); } - private Module bindings(final Path path, final ConfigModelRepo configModelRepo, final Networking networking) { + private Module bindings(Path path, ConfigModelRepo configModelRepo, Networking networking) { return new AbstractModule() { @Override protected void configure() { diff --git a/application/src/main/java/com/yahoo/application/container/Search.java b/application/src/main/java/com/yahoo/application/container/Search.java index 16db6ef4cc8..0e241c27949 100644 --- a/application/src/main/java/com/yahoo/application/container/Search.java +++ b/application/src/main/java/com/yahoo/application/container/Search.java @@ -37,11 +37,10 @@ public final class Search extends ProcessingBase<Query, Result, Searcher> { @Override protected Result doProcess(Chain<Searcher> chain, Query request) { - return handler.searchAndFill(request, chain, handler.getSearchChainRegistry()); + return handler.searchAndFill(request, chain); } @Override - @SuppressWarnings("deprecation") protected ListenableFuture<Boolean> doProcessAndRender(ComponentSpecification chainSpec, Query request, Renderer<Result> renderer, @@ -51,7 +50,6 @@ public final class Search extends ProcessingBase<Query, Result, Searcher> { } @Override - @SuppressWarnings("deprecation") protected Renderer<Result> doGetRenderer(ComponentSpecification spec) { return handler.getRendererCopy(spec); } diff --git a/application/src/test/java/com/yahoo/application/ApplicationTest.java b/application/src/test/java/com/yahoo/application/ApplicationTest.java index 3227b579832..b6e59f7d4cd 100644 --- a/application/src/test/java/com/yahoo/application/ApplicationTest.java +++ b/application/src/test/java/com/yahoo/application/ApplicationTest.java @@ -61,8 +61,9 @@ public class ApplicationTest { try (Application application = Application.fromApplicationPackage(new File("src/test/app-packages/withcontent"), Networking.disable)) { Result result = application.getJDisc("default").search().process(new ComponentSpecification("default"), - new Query("?query=substring:foobar&tracelevel=3")); - assertEquals("AND substring:fo substring:oo substring:ob substring:ba substring:ar", result.hits().get("hasQuery").getQuery().getModel().getQueryTree().toString()); + new Query("?query=substring:foobar&tracelevel=3")); + assertEquals("AND substring:fo substring:oo substring:ob substring:ba substring:ar", + result.hits().get("hasQuery").getQuery().getModel().getQueryTree().toString()); } } 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 23083bc6176..47adac637ee 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 @@ -504,7 +504,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> @Override public void getConfig(QrSearchersConfig.Builder builder) { - if (containerSearch!=null) containerSearch.getConfig(builder); + if (containerSearch != null) containerSearch.getConfig(builder); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chains.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chains.java index 0fc5c38c3b3..e6c73773840 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chains.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chains.java @@ -13,6 +13,7 @@ import java.util.Set; /** * Root config producer the whole chains model(contains chains and components). + * * @author Tony Vaagenes * @author gjoranv */ 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 52bb7b91781..da87d48b13e 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 @@ -12,7 +12,6 @@ import com.yahoo.vespa.model.container.component.Handler; * Represents a handler for processing chains. * * @author gjoranv - * @since 5.1.7 */ public class ProcessingHandler<CHAINS extends Chains<?>> extends Handler<AbstractConfigProducer<?>> 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 eea93c6b32b..b7c96d63755 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 @@ -724,9 +724,12 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } private void addSearchHandler(ApplicationContainerCluster cluster, Element searchElement) { - ProcessingHandler<SearchChains> searchHandler = new ProcessingHandler<>( - cluster.getSearch().getChains(), "com.yahoo.search.handler.SearchHandler"); + // Magic spell is needed to receive the chains config :-| + cluster.addComponent(new ProcessingHandler<>(cluster.getSearch().getChains(), + "com.yahoo.search.searchchain.ExecutionFactory")); + ProcessingHandler<SearchChains> searchHandler = new ProcessingHandler<>(cluster.getSearch().getChains(), + "com.yahoo.search.handler.SearchHandler"); String[] defaultBindings = {"http://*/search/*"}; for (String binding: serverBindings(searchElement, defaultBindings)) { searchHandler.addServerBindings(binding); diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index cca3c5c4610..a7471bc1771 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -141,7 +141,7 @@ public class HandlersConfigurerDi { } @SuppressWarnings("deprecation") - private Injector createFallbackInjector(final com.yahoo.container.Container vespaContainer, Injector discInjector) { + private Injector createFallbackInjector(com.yahoo.container.Container vespaContainer, Injector discInjector) { return discInjector.createChildInjector(new AbstractModule() { @Override protected void configure() { diff --git a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java index 0851f69a366..bd3d146cfec 100644 --- a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java +++ b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java @@ -63,6 +63,7 @@ public class CloudSubscriberFactory implements SubscriberFactory { } private static class CloudSubscriber implements Subscriber { + private final ConfigSubscriber subscriber; private final Map<ConfigKey<ConfigInstance>, ConfigHandle<ConfigInstance>> handles = new HashMap<>(); diff --git a/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java index 337997f8ac2..fde4ecfd33a 100644 --- a/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -24,6 +24,7 @@ import static com.yahoo.log.LogLevel.DEBUG; * @author ollivir */ public final class ConfigRetriever { + private static final Logger log = Logger.getLogger(ConfigRetriever.class.getName()); private final Set<ConfigKey<? extends ConfigInstance>> bootstrapKeys; @@ -33,7 +34,7 @@ public final class ConfigRetriever { private final Function<Set<ConfigKey<? extends ConfigInstance>>, Subscriber> subscribe; public ConfigRetriever(Set<ConfigKey<? extends ConfigInstance>> bootstrapKeys, - Function<Set<ConfigKey<? extends ConfigInstance>>, Subscriber> subscribe) { + Function<Set<ConfigKey<? extends ConfigInstance>>, Subscriber> subscribe) { this.bootstrapKeys = bootstrapKeys; this.componentSubscriberKeys = new HashSet<>(); this.subscribe = subscribe; diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java index 24e5dcef838..0c534b0673c 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Container.java +++ b/container-di/src/main/java/com/yahoo/container/di/Container.java @@ -190,12 +190,10 @@ public class Container { } private ComponentGraph createComponentsGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs, - long generation, Injector fallbackInjector) { - + long generation, Injector fallbackInjector) { previousConfigGeneration = generation; ComponentGraph graph = new ComponentGraph(generation); - ComponentsConfig componentsConfig = getConfig(componentsConfigKey, configsIncludingBootstrapConfigs); if (componentsConfig == null) { throw new ConfigurationRuntimeException("The set of all configs does not include a valid 'components' config. Config set: " @@ -250,7 +248,7 @@ public class Container { } public static <T extends ConfigInstance> T getConfig(ConfigKey<T> key, - Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configs) { + Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configs) { ConfigInstance inst = configs.get(key); if (inst == null || key.getConfigClass() == null) { diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java index accc844c93e..c57149748e7 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java @@ -139,7 +139,6 @@ public final class ConfiguredApplication implements Application { ContainerBuilder builder = createBuilderWithGuiceBindings(); configurer = createConfigurer(builder.guiceModules().activate()); - intitializeAndActivateContainer(builder); startReconfigurerThread(); portWatcher = new Thread(this::watchPortChange); diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 45e9d169846..80038da6750 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -4147,10 +4147,10 @@ "public" ], "methods": [ + "public void <init>(com.yahoo.statistics.Statistics, com.yahoo.jdisc.Metric, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.config.QueryProfilesConfig, com.yahoo.container.core.ContainerHttpConfig, com.yahoo.search.searchchain.ExecutionFactory)", "public void <init>(com.yahoo.container.core.ChainsConfig, com.yahoo.search.config.IndexInfoConfig, com.yahoo.container.QrSearchersConfig, com.yahoo.vespa.configdefinition.SpecialtokensConfig, com.yahoo.statistics.Statistics, com.yahoo.language.Linguistics, com.yahoo.jdisc.Metric, com.yahoo.component.provider.ComponentRegistry, java.util.concurrent.Executor, com.yahoo.container.logging.AccessLog, com.yahoo.search.query.profile.config.QueryProfilesConfig, com.yahoo.component.provider.ComponentRegistry, com.yahoo.container.core.ContainerHttpConfig)", - "protected void destroy()", "public final com.yahoo.container.jdisc.HttpResponse handle(com.yahoo.container.jdisc.HttpRequest)", - "public com.yahoo.search.Result searchAndFill(com.yahoo.search.Query, com.yahoo.component.chain.Chain, com.yahoo.search.searchchain.SearchChainRegistry)", + "public com.yahoo.search.Result searchAndFill(com.yahoo.search.Query, com.yahoo.component.chain.Chain)", "public com.yahoo.processing.rendering.Renderer getRendererCopy(com.yahoo.component.ComponentSpecification)", "public com.yahoo.search.searchchain.SearchChainRegistry getSearchChainRegistry()", "public void createRequestMapping(com.yahoo.slime.Inspector, java.util.Map, java.lang.String)" @@ -7566,6 +7566,21 @@ "public static final java.lang.String ATTRIBUTEPREFETCH" ] }, + "com.yahoo.search.searchchain.ExecutionFactory": { + "superClass": "com.yahoo.component.AbstractComponent", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(com.yahoo.container.core.ChainsConfig, com.yahoo.search.config.IndexInfoConfig, com.yahoo.container.QrSearchersConfig, com.yahoo.component.provider.ComponentRegistry, com.yahoo.vespa.configdefinition.SpecialtokensConfig, com.yahoo.language.Linguistics, com.yahoo.component.provider.ComponentRegistry)", + "public com.yahoo.search.searchchain.Execution newExecution(com.yahoo.component.chain.Chain)", + "public com.yahoo.search.searchchain.SearchChainRegistry searchChainRegistry()", + "public com.yahoo.search.rendering.RendererRegistry rendererRegistry()", + "public void deconstruct()" + ], + "fields": [] + }, "com.yahoo.search.searchchain.ForkingSearcher$CommentedSearchChain": { "superClass": "java.lang.Object", "interfaces": [], diff --git a/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java b/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java index 2fcd2466dd8..1fd8cce9889 100644 --- a/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java +++ b/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java @@ -253,11 +253,14 @@ public class IndexFacts { /** * Freeze this to prevent further changes. + * + * @return this for chaining */ - public void freeze() { + public IndexFacts freeze() { hasNGramIndices = hasNGramIndices(); // TODO: Freeze content! frozen = true; + return this; } /** Whether this contains any index which has isNGram()==true. This is free to ask on a frozen instance. */ diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index cb69f2abd07..8e654bf34b8 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -6,9 +6,6 @@ import com.yahoo.collections.Tuple2; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.Vtag; import com.yahoo.component.chain.Chain; -import com.yahoo.component.chain.ChainsConfigurer; -import com.yahoo.component.chain.model.ChainsModel; -import com.yahoo.component.chain.model.ChainsModelBuilder; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.QrSearchersConfig; import com.yahoo.container.core.ChainsConfig; @@ -24,14 +21,12 @@ import com.yahoo.language.Linguistics; import com.yahoo.log.LogLevel; import com.yahoo.net.HostName; import com.yahoo.net.UriTools; -import com.yahoo.prelude.IndexFacts; -import com.yahoo.prelude.IndexModel; import com.yahoo.prelude.query.QueryException; import com.yahoo.prelude.query.parser.ParseException; -import com.yahoo.prelude.query.parser.SpecialTokenRegistry; import com.yahoo.processing.rendering.Renderer; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.query.ranking.SoftTimeout; +import com.yahoo.search.searchchain.ExecutionFactory; import com.yahoo.slime.Inspector; import com.yahoo.slime.ObjectTraverser; import com.yahoo.vespa.config.SlimeUtils; @@ -46,7 +41,6 @@ import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; import com.yahoo.search.query.profile.config.QueryProfileConfigurer; import com.yahoo.search.query.profile.config.QueryProfilesConfig; import com.yahoo.search.query.properties.DefaultProperties; -import com.yahoo.search.rendering.RendererRegistry; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.searchchain.SearchChainRegistry; @@ -93,19 +87,9 @@ public class SearchHandler extends LoggingRequestHandler { private Value searchConnections; - private final SearchChainRegistry searchChainRegistry; - - private final RendererRegistry rendererRegistry; - - private final IndexFacts indexFacts; - - private final SpecialTokenRegistry specialTokens; - public static final String defaultSearchChainName = "default"; private static final String fallbackSearchChain = "vespa"; - private final Linguistics linguistics; - private final CompiledQueryProfileRegistry queryProfileRegistry; /** If present, responses from this will set the HTTP response header with this key to the host name of this */ @@ -113,6 +97,8 @@ public class SearchHandler extends LoggingRequestHandler { private final String selfHostname = HostName.getLocalhost(); + private final ExecutionFactory executionFactory; + private final class MeanConnections implements Callback { @Override @@ -127,31 +113,19 @@ public class SearchHandler extends LoggingRequestHandler { } @Inject - public SearchHandler(ChainsConfig chainsConfig, - IndexInfoConfig indexInfo, - QrSearchersConfig clusters, - SpecialtokensConfig specialtokens, - Statistics statistics, - Linguistics linguistics, + public SearchHandler(Statistics statistics, Metric metric, - ComponentRegistry<Renderer> renderers, Executor executor, AccessLog accessLog, QueryProfilesConfig queryProfileConfig, - ComponentRegistry<Searcher> searchers, - ContainerHttpConfig containerHttpConfig) { + ContainerHttpConfig containerHttpConfig, + ExecutionFactory executionFactory) { super(executor, accessLog, metric, true); log.log(LogLevel.DEBUG, "SearchHandler.init " + System.identityHashCode(this)); - searchChainRegistry = new SearchChainRegistry(searchers); - setupSearchChainRegistry(searchers, chainsConfig); - indexFacts = new IndexFacts(new IndexModel(indexInfo, clusters)); - indexFacts.freeze(); - specialTokens = new SpecialTokenRegistry(specialtokens); - rendererRegistry = new RendererRegistry(renderers.allComponents()); QueryProfileRegistry queryProfileRegistry = QueryProfileConfigurer.createFromConfig(queryProfileConfig); this.queryProfileRegistry = queryProfileRegistry.compile(); + this.executionFactory = executionFactory; - this.linguistics = linguistics; this.maxThreads = examineExecutor(executor); searchConnections = new Value(SEARCH_CONNECTIONS, statistics, @@ -164,16 +138,28 @@ public class SearchHandler extends LoggingRequestHandler { Optional.empty() : Optional.of( containerHttpConfig.hostResponseHeaderKey()); } - @Override - protected void destroy() { - super.destroy(); - rendererRegistry.deconstruct(); - } - - private void setupSearchChainRegistry(ComponentRegistry<Searcher> searchers, ChainsConfig chainsConfig) { - ChainsModel chainsModel = ChainsModelBuilder.buildFromConfig(chainsConfig); - ChainsConfigurer.prepareChainRegistry(searchChainRegistry, chainsModel, searchers); - searchChainRegistry.freeze(); + /** @deprecated use the other constructor */ + @Deprecated // TODO: Remove on Vespa 8 + public SearchHandler(ChainsConfig chainsConfig, + IndexInfoConfig indexInfo, + QrSearchersConfig clusters, + SpecialtokensConfig specialtokens, + Statistics statistics, + Linguistics linguistics, + Metric metric, + ComponentRegistry<Renderer> renderers, + Executor executor, + AccessLog accessLog, + QueryProfilesConfig queryProfileConfig, + ComponentRegistry<Searcher> searchers, + ContainerHttpConfig containerHttpConfig) { + this(statistics, + metric, + executor, + accessLog, + queryProfileConfig, + containerHttpConfig, + new ExecutionFactory(chainsConfig, indexInfo, clusters, searchers, specialtokens, linguistics, renderers)); } private static int examineExecutor(Executor executor) { @@ -278,7 +264,7 @@ public class SearchHandler extends LoggingRequestHandler { ErrorMessage.createInvalidQueryParameter("No search chain named '" + searchChainName + "' was found")); } else { String pathAndQuery = UriTools.rawRequest(request.getUri()); - result = search(pathAndQuery, query, searchChain, searchChainRegistry); + result = search(pathAndQuery, query, searchChain); } // Transform result to response @@ -301,7 +287,7 @@ public class SearchHandler extends LoggingRequestHandler { @NonNull private Renderer<Result> toRendererCopy(ComponentSpecification format) { - Renderer<Result> renderer = rendererRegistry.getRenderer(format); + Renderer<Result> renderer = executionFactory.rendererRegistry().getRenderer(format); renderer = perRenderingCopy(renderer); return renderer; } @@ -312,28 +298,27 @@ public class SearchHandler extends LoggingRequestHandler { chainName = defaultSearchChainName; } - Chain<Searcher> searchChain = searchChainRegistry.getChain(chainName); + Chain<Searcher> searchChain = executionFactory.searchChainRegistry().getChain(chainName); if (searchChain == null && explicitChainName == null) { // explicit chain not found should cause error chainName = fallbackSearchChain; - searchChain = searchChainRegistry.getChain(chainName); + searchChain = executionFactory.searchChainRegistry().getChain(chainName); } return new Tuple2<>(chainName, searchChain); } /** Used from container SDK, for internal use only */ - public Result searchAndFill(Query query, Chain<? extends Searcher> searchChain, SearchChainRegistry registry) { + public Result searchAndFill(Query query, Chain<? extends Searcher> searchChain) { Result errorResult = validateQuery(query); if (errorResult != null) return errorResult; - Renderer<Result> renderer = rendererRegistry.getRenderer(query.getPresentation().getRenderer()); + Renderer<Result> renderer = executionFactory.rendererRegistry().getRenderer(query.getPresentation().getRenderer()); // docsumClass null means "unset", so we set it (it might be null // here too in which case it will still be "unset" after we set it :-) if (query.getPresentation().getSummary() == null && renderer instanceof com.yahoo.search.rendering.Renderer) query.getPresentation().setSummary(((com.yahoo.search.rendering.Renderer) renderer).getDefaultSummaryClass()); - Execution execution = new Execution(searchChain, - new Execution.Context(registry, indexFacts, specialTokens, rendererRegistry, linguistics)); + Execution execution = executionFactory.newExecution(searchChain); query.getModel().setExecution(execution); execution.trace().setForceTimestamps(query.properties().getBoolean(FORCE_TIMESTAMPS, false)); if (query.properties().getBoolean(DETAILED_TIMING_LOGGING, false)) { @@ -362,7 +347,7 @@ public class SearchHandler extends LoggingRequestHandler { * For internal use only */ public Renderer<Result> getRendererCopy(ComponentSpecification spec) { - Renderer<Result> renderer = rendererRegistry.getRenderer(spec); + Renderer<Result> renderer = executionFactory.rendererRegistry().getRenderer(spec); return perRenderingCopy(renderer); } @@ -380,7 +365,7 @@ public class SearchHandler extends LoggingRequestHandler { } } - private Result search(String request, Query query, Chain<Searcher> searchChain, SearchChainRegistry registry) { + private Result search(String request, Query query, Chain<Searcher> searchChain) { if (query.getTraceLevel() >= 2) { query.trace("Invoking " + searchChain, false, 2); } @@ -393,7 +378,7 @@ public class SearchHandler extends LoggingRequestHandler { new IllegalStateException("searchConnections reference is null.")); } try { - return searchAndFill(query, searchChain, registry); + return searchAndFill(query, searchChain); } catch (ParseException e) { ErrorMessage error = ErrorMessage.createIllegalQuery("Could not parse query [" + request + "]: " + Exceptions.toMessageString(e)); @@ -502,8 +487,7 @@ public class SearchHandler extends LoggingRequestHandler { query.trace("Vespa version: " + Vtag.currentVersion.toString(), false, 4); } - public SearchChainRegistry getSearchChainRegistry() { - return searchChainRegistry; + public SearchChainRegistry getSearchChainRegistry() { return executionFactory.searchChainRegistry(); } static private String getMediaType(HttpRequest request) { diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java b/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java new file mode 100644 index 00000000000..470e74bb974 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/searchchain/ExecutionFactory.java @@ -0,0 +1,79 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.searchchain; + +import com.yahoo.component.AbstractComponent; +import com.yahoo.component.chain.Chain; +import com.yahoo.component.chain.ChainsConfigurer; +import com.yahoo.component.chain.model.ChainsModel; +import com.yahoo.component.chain.model.ChainsModelBuilder; +import com.yahoo.component.provider.ComponentRegistry; +import com.yahoo.container.QrSearchersConfig; +import com.yahoo.container.core.ChainsConfig; +import com.yahoo.language.Linguistics; +import com.yahoo.prelude.IndexFacts; +import com.yahoo.prelude.IndexModel; +import com.yahoo.prelude.query.parser.SpecialTokenRegistry; +import com.yahoo.processing.rendering.Renderer; +import com.yahoo.search.Searcher; +import com.yahoo.search.config.IndexInfoConfig; +import com.yahoo.search.rendering.RendererRegistry; +import com.yahoo.vespa.configdefinition.SpecialtokensConfig; + +/** + * Provides creation of fully configured query Execution instances. + * Have an instance of this injected if you need to execute queries which are not initiated from + * an external request. + * + * @author bratseth + */ +public class ExecutionFactory extends AbstractComponent { + + private final SearchChainRegistry searchChainRegistry; + private final IndexFacts indexFacts; + private final SpecialTokenRegistry specialTokens; + private final Linguistics linguistics; + private final RendererRegistry rendererRegistry; + + public ExecutionFactory(ChainsConfig chainsConfig, + IndexInfoConfig indexInfo, + QrSearchersConfig clusters, + ComponentRegistry<Searcher> searchers, + SpecialtokensConfig specialTokens, + Linguistics linguistics, + ComponentRegistry<Renderer> renderers) { + this.searchChainRegistry = createSearchChainRegistry(searchers, chainsConfig); + this.indexFacts = new IndexFacts(new IndexModel(indexInfo, clusters)).freeze(); + this.specialTokens = new SpecialTokenRegistry(specialTokens); + this.linguistics = linguistics; + this.rendererRegistry = new RendererRegistry(renderers.allComponents()); + } + + private SearchChainRegistry createSearchChainRegistry(ComponentRegistry<Searcher> searchers, ChainsConfig chainsConfig) { + SearchChainRegistry searchChainRegistry = new SearchChainRegistry(searchers); + ChainsModel chainsModel = ChainsModelBuilder.buildFromConfig(chainsConfig); + ChainsConfigurer.prepareChainRegistry(searchChainRegistry, chainsModel, searchers); + searchChainRegistry.freeze(); + return searchChainRegistry; + } + + /** + * Creates a new execution starting at a search chain. + * An execution instance should be used once to execute a (tree of) search chains. + */ + public Execution newExecution(Chain<? extends Searcher> searchChain) { + return new Execution(searchChain, + new Execution.Context(searchChainRegistry, indexFacts, specialTokens, rendererRegistry, linguistics)); + } + + /** Returns the search chain registry used by this */ + public SearchChainRegistry searchChainRegistry() { return searchChainRegistry; } + + /** Returns the renderers known to this */ + public RendererRegistry rendererRegistry() { return rendererRegistry; } + + @Override + public void deconstruct() { + rendererRegistry.deconstruct(); + } + +} diff --git a/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java b/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java index 87d92e1344a..02e2152d7c9 100644 --- a/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java @@ -78,8 +78,8 @@ public class JSONSearchHandlerTestCase { File activeConfig = new File(tempDir); SearchChainConfigurerTestCase. createComponentsConfig(new File(activeConfig, "chains.cfg").getPath(), - new File(activeConfig, "handlers.cfg").getPath(), - new File(activeConfig, "components.cfg").getPath()); + new File(activeConfig, "handlers.cfg").getPath(), + new File(activeConfig, "components.cfg").getPath()); } private SearchHandler fetchSearchHandler(HandlersConfigurerTestWrapper configurer) { @@ -129,7 +129,7 @@ public class JSONSearchHandlerTestCase { assertNotSame("Have a new instance of the search handler", searchHandler, newSearchHandler); assertNotNull("Have the new search chain", fetchSearchHandler(configurer).getSearchChainRegistry().getChain("hello")); assertNull("Don't have the new search chain", fetchSearchHandler(configurer).getSearchChainRegistry().getChain("classLoadingError")); - try (RequestHandlerTestDriver newDriver = new RequestHandlerTestDriver(searchHandler)) { + try (RequestHandlerTestDriver newDriver = new RequestHandlerTestDriver(newSearchHandler)) { assertJsonResult(json, newDriver); } } diff --git a/container-search/src/test/java/com/yahoo/search/handler/test/SearchHandlerTestCase.java b/container-search/src/test/java/com/yahoo/search/handler/test/SearchHandlerTestCase.java index 20b18ba6723..c96af2ed4d1 100644 --- a/container-search/src/test/java/com/yahoo/search/handler/test/SearchHandlerTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/handler/test/SearchHandlerTestCase.java @@ -36,6 +36,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -92,7 +93,7 @@ public class SearchHandlerTestCase { } @Test - public void testNullQuery() throws Exception { + public void testNullQuery() { assertEquals("<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" + "<result total-hit-count=\"0\">\n" + " <hit relevancy=\"1.0\">\n" + @@ -125,10 +126,10 @@ public class SearchHandlerTestCase { // ...and check the resulting config SearchHandler newSearchHandler = fetchSearchHandler(configurer); - assertTrue("Have a new instance of the search handler", searchHandler != newSearchHandler); + assertNotSame("Have a new instance of the search handler", searchHandler, newSearchHandler); assertNotNull("Have the new search chain", fetchSearchHandler(configurer).getSearchChainRegistry().getChain("hello")); assertNull("Don't have the new search chain", fetchSearchHandler(configurer).getSearchChainRegistry().getChain("classLoadingError")); - try (RequestHandlerTestDriver newDriver = new RequestHandlerTestDriver(searchHandler)) { + try (RequestHandlerTestDriver newDriver = new RequestHandlerTestDriver(newSearchHandler)) { assertJsonResult("http://localhost?query=abc", newDriver); } } diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/typed/components.cfg b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/typed/components.cfg index 1110d76f887..a047ae1cb73 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/typed/components.cfg +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/typed/components.cfg @@ -9,3 +9,4 @@ components[3].id DefaultSearcher components[3].classId com.yahoo.search.query.profile.config.test.QueryProfileIntegrationTestCase$DefaultSearcher components[4].id com.yahoo.search.handler.SearchHandler components[5].id com.yahoo.container.core.config.HandlersConfigurerDi$RegistriesHack +components[6].id com.yahoo.search.searchchain.ExecutionFactory diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/untyped/components.cfg b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/untyped/components.cfg index 70f5452b74d..ef9d4490a77 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/untyped/components.cfg +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/untyped/components.cfg @@ -1,4 +1,4 @@ -components[6] +components[7] components[0].id SettingSearcher components[0].classId com.yahoo.search.query.profile.config.test.QueryProfileIntegrationTestCase$SettingSearcher components[1].id ReceivingSearcher @@ -8,4 +8,5 @@ components[2].classId com.yahoo.search.query.profile.config.test.QueryProfileInt components[3].id DefaultSearcher components[3].classId com.yahoo.search.query.profile.config.test.QueryProfileIntegrationTestCase$DefaultSearcher components[4].id com.yahoo.search.handler.SearchHandler -components[5].id com.yahoo.container.core.config.HandlersConfigurerDi$RegistriesHack
\ No newline at end of file +components[5].id com.yahoo.container.core.config.HandlersConfigurerDi$RegistriesHack +components[6].id com.yahoo.search.searchchain.ExecutionFactory diff --git a/container-search/src/test/java/com/yahoo/search/searchchain/config/test/SearchChainConfigurerTestCase.java b/container-search/src/test/java/com/yahoo/search/searchchain/config/test/SearchChainConfigurerTestCase.java index 699e8619cd0..a57ed07017f 100644 --- a/container-search/src/test/java/com/yahoo/search/searchchain/config/test/SearchChainConfigurerTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/searchchain/config/test/SearchChainConfigurerTestCase.java @@ -10,6 +10,7 @@ import com.yahoo.search.Result; import com.yahoo.search.Searcher; import com.yahoo.search.handler.SearchHandler; import com.yahoo.search.searchchain.Execution; +import com.yahoo.search.searchchain.ExecutionFactory; import com.yahoo.search.searchchain.SearchChain; import com.yahoo.search.searchchain.SearchChainRegistry; import com.yahoo.search.searchchain.SearcherRegistry; @@ -309,6 +310,7 @@ public class SearchChainConfigurerTestCase { /** * Copies the component ids from another config, e.g. 'handlers' to a 'components' array in a new components file, * to avoid a manually written 'components' file for tests where the bundle spec is given by the component id. + * * @param configFile Full path to the original config file, e.g. 'handlers' * @param componentsFile Full path to the new 'components' file * @param componentType The type of component, e.g. 'handler' @@ -324,7 +326,7 @@ public class SearchChainConfigurerTestCase { if (append) { Pattern p = Pattern.compile("^[a-z]+" + "\\[\\d+\\]\\.id (.+)"); BufferedReader reader = new BufferedReader(new InputStreamReader( - new FileInputStream(new File(componentsFile)), "UTF-8")); + new FileInputStream(new File(componentsFile)), "UTF-8")); while ((line = reader.readLine()) != null) { Matcher m = p.matcher(line); if (m.matches() && !m.group(1).equals(HandlersConfigurerDi.RegistriesHack.class.getName())) { @@ -344,10 +346,11 @@ public class SearchChainConfigurerTestCase { i++; } } - buf.append("components[").append(i).append("].id "). - append(HandlersConfigurerDi.RegistriesHack.class.getName()).append("\n"); - i++; reader.close(); + + buf.append("components[").append(i++).append("].id ").append(HandlersConfigurerDi.RegistriesHack.class.getName()).append("\n"); + if (componentType.equals("components")) + buf.append("components[").append(i++).append("].id ").append(ExecutionFactory.class.getName()).append("\n"); buf.insert(0, "components["+i+"]\n"); Writer writer = new OutputStreamWriter(new FileOutputStream(new File(componentsFile)), "UTF-8"); diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java index 6ea2671b05b..d65b41c11c7 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java @@ -52,9 +52,8 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { Map<ConfigKey<ConfigInstance>, ConfigInstance> ret = new HashMap<>(); for (ConfigKey<ConfigInstance> key : configKeys) { ConfigInstance.Builder builder = root.getConfig(newBuilderInstance(key), key.getConfigId()); - if (builder == null) { + if (builder == null) throw new RuntimeException("Invalid config id " + key.getConfigId()); - } ret.put(key, newConfigInstance(builder)); } return ret; |