From cc0ca27d5ac61452397e6931fdf6a3fc3033a29e Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 11 Jul 2022 08:30:48 +0200 Subject: Add deploy warnings for deprecated and ignored docproc config Also cleanup dead code related to the above. --- .../model/container/docproc/ContainerDocproc.java | 48 ++++--------- .../model/container/xml/ContainerModelBuilder.java | 2 +- .../model/container/xml/DocprocOptionsBuilder.java | 55 ++++++--------- .../vespa/model/routing/DocumentProtocol.java | 79 ++++++++-------------- 4 files changed, 65 insertions(+), 119 deletions(-) (limited to 'config-model/src/main/java/com') diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/ContainerDocproc.java b/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/ContainerDocproc.java index fee10b965aa..242f7cbefdb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/ContainerDocproc.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/docproc/ContainerDocproc.java @@ -15,35 +15,26 @@ import java.util.HashMap; import java.util.Map; /** - * @author einarmr + * @author Einar M R Rosenvinge * @author gjoranv */ -public class ContainerDocproc extends ContainerSubsystem - implements - ContainerMbusConfig.Producer, - SchemamappingConfig.Producer, - DocprocConfig.Producer -{ - public final Options options; - - // Whether or not to prefer sending to a local node. - private final boolean preferLocalNode = false; - - // The number of nodes to use per client. - private final int numNodesPerClient = 0; +public class ContainerDocproc extends ContainerSubsystem implements + ContainerMbusConfig.Producer, + SchemamappingConfig.Producer, + DocprocConfig.Producer { + public final Options options; private final Map, String> fieldNameSchemaMap = new HashMap<>(); public ContainerDocproc(ContainerCluster cluster, DocprocChains chains) { - this(cluster, chains, new Options( null, null, null, null, null, null)); + this(cluster, chains, Options.empty()); } public ContainerDocproc(ContainerCluster cluster, DocprocChains chains, Options options) { this(cluster, chains, options, true); } - private void addSource( - final ContainerCluster cluster, final String name, final SessionConfig.Type.Enum type) { + private void addSource(ContainerCluster cluster, String name, SessionConfig.Type.Enum type) { final MbusClient mbusClient = new MbusClient(name, type); mbusClient.addClientBindings(SystemBindingPattern.fromPattern("mbus://*/" + mbusClient.getSessionName())); cluster.addComponent(mbusClient); @@ -61,14 +52,6 @@ public class ContainerDocproc extends ContainerSubsystem cluster.addSearchAndDocprocBundles(); } - public boolean isPreferLocalNode() { - return preferLocalNode; - } - - public int getNumNodesPerClient() { - return numNodesPerClient; - } - @Override public void getConfig(ContainerMbusConfig.Builder builder) { builder.maxpendingcount(getMaxMessagesInQueue()); @@ -79,13 +62,9 @@ public class ContainerDocproc extends ContainerSubsystem return options.maxMessagesInQueue; } - //maxmessagesinqueue has not been set for this node. let's try to give a good value anyway: + // maxmessagesinqueue has not been set for this node. let's try to give a good value anyway: return 2048 * getChains().allChains().allComponents().size(); - //intentionally high, getMaxQueueMbSize() will probably kick in before this one! - } - - private Integer getMaxQueueMbSize() { - return options.maxQueueMbSize; + // intentionally high, getMaxQueueMbSize() will probably kick in before this one! } private Integer getMaxQueueTimeMs() { @@ -134,21 +113,22 @@ public class ContainerDocproc extends ContainerSubsystem public static class Options { public final Integer maxMessagesInQueue; - public final Integer maxQueueMbSize; public final Integer maxQueueTimeMs; public final Double maxConcurrentFactor; public final Double documentExpansionFactor; public final Integer containerCoreMemory; - public Options(Integer maxMessagesInQueue, Integer maxQueueMbSize, Integer maxQueueTimeMs, Double maxConcurrentFactor, Double documentExpansionFactor, Integer containerCoreMemory) { + public Options(Integer maxMessagesInQueue, Integer maxQueueTimeMs, Double maxConcurrentFactor, Double documentExpansionFactor, Integer containerCoreMemory) { this.maxMessagesInQueue = maxMessagesInQueue; - this.maxQueueMbSize = maxQueueMbSize; this.maxQueueTimeMs = maxQueueTimeMs; this.maxConcurrentFactor = maxConcurrentFactor; this.documentExpansionFactor = documentExpansionFactor; this.containerCoreMemory = containerCoreMemory; } + + static Options empty() { return new Options(null, null, null, null, null); } + } } 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 1c47f1d7c9c..6d0b5085ce6 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 @@ -940,7 +940,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { addIncludes(docprocElement); DocprocChains chains = new DomDocprocChainsBuilder(null, false).build(deployState, cluster, docprocElement); - ContainerDocproc.Options docprocOptions = DocprocOptionsBuilder.build(docprocElement); + ContainerDocproc.Options docprocOptions = DocprocOptionsBuilder.build(docprocElement, deployState.getDeployLogger()); return new ContainerDocproc(cluster, chains, docprocOptions, !standaloneBuilder); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/DocprocOptionsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/DocprocOptionsBuilder.java index e0dffca2bc5..faf2d01d385 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/DocprocOptionsBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/DocprocOptionsBuilder.java @@ -1,17 +1,18 @@ // 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.config.application.api.DeployLogger; import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import org.w3c.dom.Element; +import java.util.Set; +import java.util.logging.Level; -/** - * Extracted from DomDocProcClusterBuilder - */ public class DocprocOptionsBuilder { - public static ContainerDocproc.Options build(Element spec) { + public static ContainerDocproc.Options build(Element spec, DeployLogger deployLogger) { + checkForDeprecatedAttributes(spec, Set.of("maxqueuebytesize", "numnodesperclient", "preferlocalnode"), deployLogger); + return new ContainerDocproc.Options( getMaxMessagesInQueue(spec), - getSizeInMegabytes(spec.getAttribute("maxqueuebytesize")), getTime(spec.getAttribute("maxqueuewait")), getFactor(spec.getAttribute("maxconcurrentfactor")), getFactor(spec.getAttribute("documentexpansionfactor")), @@ -19,18 +20,17 @@ public class DocprocOptionsBuilder { } private static Integer getInt(String integer) { - return integer == null || integer.trim().isEmpty() ? - null: - Integer.parseInt(integer); + return integer == null || integer.trim().isEmpty() + ? null + : Integer.parseInt(integer); } private static Double getFactor(String factor) { - return factor == null || factor.trim().isEmpty() ? - null : - Double.parseDouble(factor); + return factor == null || factor.trim().isEmpty() + ? null + : Double.parseDouble(factor); } - private static Integer getMaxMessagesInQueue(Element spec) { // get max queue size (number of messages), if set Integer maxMessagesInQueue = null; @@ -40,28 +40,6 @@ public class DocprocOptionsBuilder { return maxMessagesInQueue; } - private static Integer getSizeInMegabytes(String size) { - if (size == null) { - return null; - } - size = size.trim(); - if (size.isEmpty()) { - return null; - } - - Integer megabyteSize; - if (size.endsWith("m")) { - size = size.substring(0, size.length() - 1); - megabyteSize = Integer.parseInt(size); - } else if (size.endsWith("g")) { - size = size.substring(0, size.length() - 1); - megabyteSize = Integer.parseInt(size) * 1024; - } else { - throw new IllegalArgumentException("Heap sizes for docproc must be set to Xm or Xg, where X is an integer specifying megabytes or gigabytes, respectively."); - } - return megabyteSize; - } - private static Integer getTime(String intStr) { if (intStr == null) { return null; @@ -73,4 +51,13 @@ public class DocprocOptionsBuilder { return 1000 * (int)Double.parseDouble(intStr); } + + private static void checkForDeprecatedAttributes(Element spec, Set names, DeployLogger deployLogger) { + names.forEach(n -> { + if (!spec.getAttribute(n).isEmpty()) + deployLogger.logApplicationPackage(Level.WARNING, "'" + n + "' is ignored, deprecated and will be removed in Vespa 9."); + }); + } + + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java b/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java index 754800a42e1..8a8d38e23e3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/routing/DocumentProtocol.java @@ -182,44 +182,26 @@ public final class DocumentProtocol implements Protocol, return table; } - private static void addContainerClusterDocprocHops(Collection> containerClusters, - RoutingTableSpec table) { - + private static void addContainerClusterDocprocHops(Collection> containerClusters, RoutingTableSpec table) { for (ContainerCluster cluster: containerClusters) { ContainerDocproc docproc = cluster.getDocproc(); if (docproc != null) { - String policy = policy(docproc); - for (DocprocChain chain : docproc.getChains().allChains().allComponents()) { - addChainHop(table, cluster.getConfigId(), policy, chain); + addChainHop(table, cluster.getConfigId(), chain); } } } } - private static void addChainHop(RoutingTableSpec table, String configId, String policy, DocprocChain chain) { - final StringBuilder selector = new StringBuilder(); - if (policy != null) { - selector.append(configId).append("/").append(policy).append("/").append(chain.getSessionName()); - } else { - selector.append("[LoadBalancer:cluster=").append(configId) - .append(";session=").append(chain.getSessionName()) - .append("]"); - } + private static void addChainHop(RoutingTableSpec table, String configId, DocprocChain chain) { + StringBuilder selector = new StringBuilder(); + selector.append("[LoadBalancer:cluster=").append(configId) + .append(";session=").append(chain.getSessionName()) + .append("]"); table.addHop(new HopSpec(chain.getServiceName(), selector.toString())); } - private static String policy(ContainerDocproc docproc) { - if (docproc.getNumNodesPerClient() > 0) { - return "[SubsetService:" + docproc.getNumNodesPerClient() + "]"; - } else if (docproc.isPreferLocalNode()) { - return "[LocalService]"; - } else { - return null; - } - } - /** * Create hops to all configured storage nodes for the Document protocol. The "Distributor" policy resolves its * recipients using slobrok lookups, so it requires no configured recipients. @@ -228,23 +210,22 @@ public final class DocumentProtocol implements Protocol, * @param table the routing table to add to */ private static void addContentRouting(List content, RoutingTableSpec table) { - - for (ContentCluster cluster : content) { - RouteSpec spec = new RouteSpec(cluster.getConfigId()); - - if (cluster.getSearch().hasIndexedCluster()) { - table.addRoute(spec.addHop("[MessageType:" + cluster.getConfigId() + "]")); - table.addRoute(new RouteSpec(getIndexedRouteName(cluster.getConfigId())) - .addHop(cluster.getSearch().getIndexed().getIndexingServiceName()) - .addHop("[Content:cluster=" + cluster.getName() + "]")); - table.addRoute(new RouteSpec(getDirectRouteName(cluster.getConfigId())) - .addHop("[Content:cluster=" + cluster.getName() + "]")); - } else { - table.addRoute(spec.addHop("[Content:cluster=" + cluster.getName() + "]")); - } - table.addRoute(new RouteSpec("storage/cluster." + cluster.getName()) - .addHop("route:" + cluster.getConfigId())); + for (ContentCluster cluster : content) { + RouteSpec spec = new RouteSpec(cluster.getConfigId()); + + if (cluster.getSearch().hasIndexedCluster()) { + table.addRoute(spec.addHop("[MessageType:" + cluster.getConfigId() + "]")); + table.addRoute(new RouteSpec(getIndexedRouteName(cluster.getConfigId())) + .addHop(cluster.getSearch().getIndexed().getIndexingServiceName()) + .addHop("[Content:cluster=" + cluster.getName() + "]")); + table.addRoute(new RouteSpec(getDirectRouteName(cluster.getConfigId())) + .addHop("[Content:cluster=" + cluster.getName() + "]")); + } else { + table.addRoute(spec.addHop("[Content:cluster=" + cluster.getName() + "]")); } + table.addRoute(new RouteSpec("storage/cluster." + cluster.getName()) + .addHop("route:" + cluster.getConfigId())); + } } /** @@ -255,9 +236,8 @@ public final class DocumentProtocol implements Protocol, * @param table the routing table to add to */ private static void addIndexingHop(List content, RoutingTableSpec table) { - if (content.isEmpty()) { - return; - } + if (content.isEmpty()) return; + HopSpec hop = new HopSpec("indexing", "[DocumentRouteSelector]"); for (ContentCluster cluster : content) { hop.addRecipient(cluster.getConfigId()); @@ -281,9 +261,8 @@ public final class DocumentProtocol implements Protocol, private static void addDefaultRoutes(List content, Collection> containerClusters, RoutingTableSpec table) { - if (content.isEmpty() || !indexingHopExists(table)) { - return; - } + if (content.isEmpty() || !indexingHopExists(table)) return; + RouteSpec route = new RouteSpec("default"); String hop = getContainerClustersDocprocHop(containerClusters); if (hop != null) { @@ -326,9 +305,9 @@ public final class DocumentProtocol implements Protocol, } private static DocprocChain getDefaultChain(ContainerDocproc docproc) { - return docproc == null ? - null: - docproc.getChains().allChains().getComponent("default"); + return docproc == null + ? null + : docproc.getChains().allChains().getComponent("default"); } /** -- cgit v1.2.3