diff options
75 files changed, 1076 insertions, 1134 deletions
diff --git a/bootstrap.sh b/bootstrap.sh index 6cdcc17400c..c8f300408db 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -25,8 +25,24 @@ else exit 1 fi +get_env_var_with_optional_default() { + local var_name=$1 + local default_val=$2 + eval "existing_value=\${$var_name}" + if [[ -n $existing_value ]]; then + echo "$existing_value" + elif [[ -n $default_val ]]; then + echo "$default_val" + fi +} + +readonly MAVEN_CMD=$(get_env_var_with_optional_default VESPA_MAVEN_COMMAND mvn) +readonly MAVEN_EXTRA_OPTS=$(get_env_var_with_optional_default VESPA_MAVEN_EXTRA_OPTS) +echo "Using maven command: ${MAVEN_CMD}" +echo "Using maven extra opts: ${MAVEN_EXTRA_OPTS}" + mvn_install() { - mvn --quiet --batch-mode --no-snapshot-updates clean install -Dmaven.javadoc.skip=true "$@" + ${MAVEN_CMD} --quiet --batch-mode --no-snapshot-updates clean install -Dmaven.javadoc.skip=true ${MAVEN_EXTRA_OPTS} "$@" } # Generate vtag map diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java index e9abcfd87a9..20415343755 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java @@ -169,9 +169,7 @@ public class DomConfigPayloadBuilder { // Check for legacy (pre Vespa 6) usage throw new IllegalArgumentException("The 'index' attribute on config elements is not supported - use <item>"); } else if (element.hasAttribute("operation")) { - // inner array, currently the only supported operation is 'append' - verifyLegalOperation(element); - ConfigPayloadBuilder childPayloadBuilder = payloadBuilder.getArray(name).append(); + ConfigPayloadBuilder childPayloadBuilder = getBuilderForInnerArray(element, payloadBuilder, name); //Cursor array = node.setArray(name); for (Element child : children) { //Cursor struct = array.addObject(); @@ -240,4 +238,24 @@ public class DomConfigPayloadBuilder { + operation + "' at XML node '" + XML.getNodePath(currElem, " > ") + "'."); } + private ConfigPayloadBuilder getBuilderForInnerArray(Element element, ConfigPayloadBuilder payloadBuilder, String arrayName) { + // inner array, the supported operations are 'append' and 'clear' + String operation = element.getAttribute("operation").toLowerCase(); + ConfigPayloadBuilder arrayPayloadBuilder; + switch (operation) { + case "append": + arrayPayloadBuilder = payloadBuilder.getArray(arrayName).append(); + break; + case "clear": + // Clear array if it exists, use the existing builder + // Creating the array happens when handling the children ('item's) + payloadBuilder.removeArray(arrayName); + arrayPayloadBuilder = payloadBuilder; + break; + default: + throw new RuntimeException("Unknown operation '" + operation + "' at XML node '" + XML.getNodePath(element, " > ") + "'."); + } + return arrayPayloadBuilder; + } + } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java index d1ef1010b73..747ce93d86b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java @@ -8,7 +8,6 @@ import com.yahoo.config.model.deploy.ConfigDefinitionStore; import com.yahoo.test.SimpletypesConfig; import com.yahoo.config.model.producer.UserConfigRepo; import com.yahoo.config.model.builder.xml.XmlHelper; -import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigDefinitionKey; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.ConfigPayloadBuilder; @@ -17,30 +16,28 @@ import org.junit.Test; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.InputSource; -import org.xml.sax.SAXException; -import javax.xml.parsers.ParserConfigurationException; -import java.io.IOException; import java.io.Reader; import java.io.StringReader; +import java.util.Arrays; +import java.util.Collections; import java.util.Optional; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; /** * @author Ulf Lilleengen - * @since 5.1 */ public class UserConfigBuilderTest { - private final ConfigDefinitionStore configDefinitionStore = new ConfigDefinitionStore() { - @Override - public Optional<ConfigDefinition> getConfigDefinition(ConfigDefinitionKey defKey) { return Optional.empty(); } - }; + private final ConfigDefinitionStore configDefinitionStore = defKey -> Optional.empty(); @Test - public void require_that_simple_config_is_resolved() throws ParserConfigurationException, IOException, SAXException { + public void require_that_simple_config_is_resolved() { Element configRoot = getDocument("<config name=\"simpletypes\">" + " <intval>13</intval>" + "</config>" + @@ -56,31 +53,59 @@ public class UserConfigBuilderTest { assertThat(config.stringval(), is("foolio")); } - public static <ConfigType extends ConfigInstance> ConfigType createConfig(Class<ConfigType> clazz, ConfigPayloadBuilder builder) { - return ConfigPayload.fromBuilder(builder).toInstance(clazz, ""); - } - - @Test - public void require_that_arrays_config_is_resolved() throws ParserConfigurationException, IOException, SAXException { + public void require_that_arrays_config_is_resolved() { Element configRoot = getDocument("<config name=\"arraytypes\">" + - " <intarr operation=\"append\">13</intarr>" + - " <intarr operation=\"append\">10</intarr>" + - " <intarr operation=\"append\">1337</intarr>" + - "</config>"); + " <intarr operation=\"append\">13</intarr>" + + " <intarr operation=\"append\">10</intarr>" + + " <intarr operation=\"append\">1337</intarr>" + + "</config>"); UserConfigRepo map = UserConfigBuilder.build(configRoot, configDefinitionStore, new BaseDeployLogger()); assertFalse(map.isEmpty()); ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config"); assertNotNull(map.get(key)); ArraytypesConfig config = createConfig(ArraytypesConfig.class, map.get(key)); - assertThat(config.intarr().size(), is(3)); - assertThat(config.intarr(0), is(13)); - assertThat(config.intarr(1), is(10)); - assertThat(config.intarr(2), is(1337)); + assertEquals(Arrays.asList(13,10,1337), config.intarr()); } @Test - public void require_that_arrays_of_structs_are_resolved() throws ParserConfigurationException, IOException, SAXException { + public void require_that_array_with_items_is_resolved() { + ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config"); + + Element configRoot = getDocument("<config name='arraytypes'>" + + " <intarr operation='clear'>" + + " <item>0</item>" + + " </intarr>" + + "</config>"); + UserConfigRepo map = UserConfigBuilder.build(configRoot, configDefinitionStore, new BaseDeployLogger()); + assertFalse(map.isEmpty()); + assertNotNull(map.get(key)); + ArraytypesConfig config = createConfig(ArraytypesConfig.class, map.get(key)); + assertEquals(Collections.singletonList(0), config.intarr()); + + Element configRoot2 = getDocument("<config name='arraytypes'>" + + " <intarr>" + + " <item>1</item>" + + " <item>2</item>" + + " </intarr>" + + "</config>"); + UserConfigRepo map2 = mergeAndCreateConfig(map, configRoot2); + ArraytypesConfig config2 = createConfig(ArraytypesConfig.class, map2.get(key)); + assertEquals(Arrays.asList(1, 2), config2.intarr()); + + + Element configRoot3 = getDocument("<config name='arraytypes'>" + + " <intarr operation='clear'>" + + " <item>3</item>" + + " </intarr>" + + "</config>"); + UserConfigRepo map3 = mergeAndCreateConfig(map2, configRoot3); + ArraytypesConfig config3 = createConfig(ArraytypesConfig.class, map3.get(key)); + assertEquals(Collections.singletonList(3), config3.intarr()); + } + + @Test + public void require_that_arrays_of_structs_are_resolved() { Element configRoot = getDocument( " <config name='vespa.configdefinition.specialtokens'>" + " <tokenlist operation='append'>" + @@ -107,7 +132,7 @@ public class UserConfigBuilderTest { } @Test - public void no_exception_when_config_class_does_not_exist() throws ParserConfigurationException, IOException, SAXException { + public void no_exception_when_config_class_does_not_exist() { Element configRoot = getDocument("<config name=\"unknown\">" + " <foo>1</foo>" + "</config>"); @@ -116,7 +141,7 @@ public class UserConfigBuilderTest { assertNotNull(builder); } - private Element getDocument(String xml) throws ParserConfigurationException { + private Element getDocument(String xml) { Reader xmlReader = new StringReader("<model>" + xml + "</model>"); Document doc; try { @@ -126,4 +151,15 @@ public class UserConfigBuilderTest { } return doc.getDocumentElement(); } + + private static <ConfigType extends ConfigInstance> ConfigType createConfig(Class<ConfigType> clazz, ConfigPayloadBuilder builder) { + return ConfigPayload.fromBuilder(builder).toInstance(clazz, ""); + } + + private UserConfigRepo mergeAndCreateConfig(UserConfigRepo original, Element newElement) { + UserConfigRepo map = UserConfigBuilder.build(newElement, configDefinitionStore, new BaseDeployLogger()); + original.merge(map); + return map; + } + } diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java index bb974ddae42..863505af5b0 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java +++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java @@ -109,6 +109,15 @@ public class ConfigPayloadBuilder { return a; } + /** + * Removes an array + * + * @param name Name of array. + */ + public void removeArray(String name) { + arrayMap.remove(name); + } + private void validateArray(String name) { if (configDefinition != null) { configDefinition.verify(name); @@ -417,6 +426,10 @@ public class ConfigPayloadBuilder { } return this; } + + public void clear() { + elements.clear(); + } } private ConfigPayloadBuilder(ConfigPayloadBuilder other) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index a5e4cfede0e..bd5b694d657 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -6,7 +6,6 @@ import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.log.LogLevel; import com.yahoo.path.Path; @@ -182,7 +181,7 @@ public class TenantRepository implements ConnectionStateListener, PathChildrenCa } } - if (failed.size() > 0 && throwExceptionIfBootstrappingFails && globalComponentRegistry.getZone().system() == SystemName.cd) + if (failed.size() > 0 && throwExceptionIfBootstrappingFails) throw new RuntimeException("Could not create all tenants when bootstrapping, failed to create: " + failed); metricUpdater.setTenants(tenants.size()); diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index 4859222d69a..1376b50dad9 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -27,9 +27,11 @@ import java.util.Map; import static com.yahoo.container.jdisc.state.StateHandler.getSnapshotPreprocessor; /** - * This handler outputs metrics in a json-like format. Each individual metric is a json object (packet), - * but there is no outer array or object that wraps the metrics packets. This handler is not set up by - * default, but can be added to the applications's services configuration. + * This handler outputs metrics in a json-like format, consisting of a series of metrics packets. + * Each packet is a json object but there is no outer array or object that wraps the packets. + * To reduce the amount of output, a packet contains all metrics that share the same set of dimensions. + * + * This handler is not set up by default, but can be added to the applications's services configuration. * * This handler is protocol agnostic, so it cannot discriminate between e.g. http request * methods (get/head/post etc.). @@ -87,7 +89,7 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { private byte[] buildMetricOutput() { try { - String output = getStatusPacket() + getAllMetricsPackets(); + String output = getStatusPacket() + getAllMetricsPackets() + "\n"; return output.getBytes(StandardCharsets.UTF_8); } catch (JSONException e) { throw new RuntimeException("Bad JSON construction.", e); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java index f68bb718c8d..51048db3cb7 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java @@ -8,10 +8,12 @@ import com.yahoo.search.Result; import com.yahoo.search.dispatch.FillInvoker; import com.yahoo.search.dispatch.InterleavedFillInvoker; import com.yahoo.search.dispatch.InterleavedSearchInvoker; -import com.yahoo.search.dispatch.SearchCluster; import com.yahoo.search.dispatch.SearchInvoker; +import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.search.result.Hit; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -30,18 +32,20 @@ import java.util.Set; public class FS4InvokerFactory { private final FS4ResourcePool fs4ResourcePool; private final VespaBackEndSearcher searcher; - private final ImmutableMap<Integer, SearchCluster.Node> nodesByKey; + private final SearchCluster searchCluster; + private final ImmutableMap<Integer, Node> nodesByKey; public FS4InvokerFactory(FS4ResourcePool fs4ResourcePool, SearchCluster searchCluster, VespaBackEndSearcher searcher) { this.fs4ResourcePool = fs4ResourcePool; this.searcher = searcher; + this.searchCluster = searchCluster; - ImmutableMap.Builder<Integer, SearchCluster.Node> builder = ImmutableMap.builder(); + ImmutableMap.Builder<Integer, Node> builder = ImmutableMap.builder(); searchCluster.groups().values().forEach(group -> group.nodes().forEach(node -> builder.put(node.key(), node))); this.nodesByKey = builder.build(); } - public SearchInvoker getSearchInvoker(Query query, SearchCluster.Node node) { + public SearchInvoker getSearchInvoker(Query query, Node node) { Backend backend = fs4ResourcePool.getBackend(node.hostname(), node.fs4port()); return new FS4SearchInvoker(searcher, query, backend.openChannel(), node); } @@ -49,22 +53,46 @@ public class FS4InvokerFactory { /** * Create a {@link SearchInvoker} for a list of content nodes. * - * @param query the search query being processed - * @param nodes pre-selected list of content nodes - * @return Optional containing the SearchInvoker or <i>empty</i> if some node in the list is invalid + * @param query + * the search query being processed + * @param groupId + * the id of the node group to which the nodes belong + * @param nodes + * pre-selected list of content nodes + * @param acceptIncompleteCoverage + * if some of the nodes are unavailable and this parameter is + * <b>false</b>, verify that the remaining set of nodes has enough + * coverage + * @return Optional containing the SearchInvoker or <i>empty</i> if some node in the + * list is invalid and the remaining coverage is not sufficient */ - public Optional<SearchInvoker> getSearchInvoker(Query query, List<SearchCluster.Node> nodes) { + public Optional<SearchInvoker> getSearchInvoker(Query query, int groupId, List<Node> nodes, boolean acceptIncompleteCoverage) { Map<Integer, SearchInvoker> invokers = new HashMap<>(); - for (SearchCluster.Node node : nodes) { + Set<Integer> failed = null; + for (Node node : nodes) { if (node.isWorking()) { Backend backend = fs4ResourcePool.getBackend(node.hostname(), node.fs4port()); if (backend.probeConnection()) { invokers.put(node.key(), new FS4SearchInvoker(searcher, query, backend.openChannel(), node)); } else { - return Optional.empty(); + if(failed == null) { + failed = new HashSet<>(); + } + failed.add(node.key()); } } } + if (failed != null && ! acceptIncompleteCoverage) { + List<Node> success = new ArrayList<>(nodes.size() - failed.size()); + for (Node node : nodes) { + if (!failed.contains(node.key())) { + success.add(node); + } + } + if (!searchCluster.isPartialGroupCoverageSufficient(groupId, success)) { + return Optional.empty(); + } + } if (invokers.size() == 1) { return Optional.of(invokers.values().iterator().next()); } else { @@ -72,7 +100,7 @@ public class FS4InvokerFactory { } } - public FillInvoker getFillInvoker(Query query, SearchCluster.Node node) { + public FillInvoker getFillInvoker(Query query, Node node) { return new FS4FillInvoker(searcher, query, fs4ResourcePool, node.hostname(), node.fs4port(), node.key()); } @@ -88,7 +116,7 @@ public class FS4InvokerFactory { Query query = result.getQuery(); Map<Integer, FillInvoker> invokers = new HashMap<>(); for (Integer distKey : requiredNodes) { - SearchCluster.Node node = nodesByKey.get(distKey); + Node node = nodesByKey.get(distKey); if (node == null) { return Optional.empty(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java index dc8cd53e638..98676890bdf 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java @@ -11,8 +11,8 @@ import com.yahoo.fs4.mplex.FS4Channel; import com.yahoo.fs4.mplex.InvalidChannelException; import com.yahoo.search.Query; import com.yahoo.search.Result; -import com.yahoo.search.dispatch.SearchCluster; import com.yahoo.search.dispatch.SearchInvoker; +import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; @@ -33,13 +33,13 @@ import static java.util.Arrays.asList; public class FS4SearchInvoker extends SearchInvoker { private final VespaBackEndSearcher searcher; private FS4Channel channel; - private final Optional<SearchCluster.Node> node; + private final Optional<Node> node; private ErrorMessage pendingSearchError = null; private Query query = null; private QueryPacket queryPacket = null; - public FS4SearchInvoker(VespaBackEndSearcher searcher, Query query, FS4Channel channel, SearchCluster.Node node) { + public FS4SearchInvoker(VespaBackEndSearcher searcher, Query query, FS4Channel channel, Node node) { this.searcher = searcher; this.node = Optional.of(node); this.channel = channel; @@ -115,7 +115,7 @@ public class FS4SearchInvoker extends SearchInvoker { searcher.addMetaInfo(query, queryPacket.getQueryPacketData(), resultPacket, result); - searcher.addUnfilledHits(result, resultPacket.getDocuments(), false, queryPacket.getQueryPacketData(), cacheKey, node.map(SearchCluster.Node::key)); + searcher.addUnfilledHits(result, resultPacket.getDocuments(), false, queryPacket.getQueryPacketData(), cacheKey, node.map(Node::key)); Packet[] packets; CacheControl cacheControl = searcher.getCacheControl(); PacketWrapper packetWrapper = cacheControl.lookup(cacheKey, query); @@ -130,7 +130,7 @@ public class FS4SearchInvoker extends SearchInvoker { } else { packets = new Packet[1]; packets[0] = resultPacket; - cacheControl.cache(cacheKey, query, new DocsumPacketKey[0], packets, node.map(SearchCluster.Node::key)); + cacheControl.cache(cacheKey, query, new DocsumPacketKey[0], packets, node.map(Node::key)); } } return asList(result); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index 36d283040a2..11b71c2c159 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -17,8 +17,8 @@ import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.Dispatcher; import com.yahoo.search.dispatch.FillInvoker; -import com.yahoo.search.dispatch.SearchCluster; import com.yahoo.search.dispatch.SearchInvoker; +import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.grouping.GroupingRequest; import com.yahoo.search.grouping.request.GroupingOperation; import com.yahoo.search.query.Ranking; @@ -218,7 +218,7 @@ public class FastSearcher extends VespaBackEndSearcher { return invoker.get(); } - Optional<SearchCluster.Node> direct = getDirectNode(query); + Optional<Node> direct = getDirectNode(query); if(direct.isPresent()) { return fs4InvokerFactory.getSearchInvoker(query, direct.get()); } @@ -237,7 +237,7 @@ public class FastSearcher extends VespaBackEndSearcher { return invoker.get(); } - Optional<SearchCluster.Node> direct = getDirectNode(query); + Optional<Node> direct = getDirectNode(query); if (direct.isPresent()) { return fs4InvokerFactory.getFillInvoker(query, direct.get()); } @@ -248,18 +248,18 @@ public class FastSearcher extends VespaBackEndSearcher { * If the query can be directed to a single local content node, returns that node. Otherwise, * returns an empty value. */ - private Optional<SearchCluster.Node> getDirectNode(Query query) { + private Optional<Node> getDirectNode(Query query) { if (!query.properties().getBoolean(dispatchDirect, true)) return Optional.empty(); if (query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) return Optional.empty(); - Optional<SearchCluster.Node> directDispatchRecipient = dispatcher.searchCluster().directDispatchTarget(); + Optional<Node> directDispatchRecipient = dispatcher.searchCluster().directDispatchTarget(); if (!directDispatchRecipient.isPresent()) return Optional.empty(); // Dispatch directly to the single, local search node - SearchCluster.Node local = directDispatchRecipient.get(); + Node local = directDispatchRecipient.get(); query.trace(false, 2, "Dispatching directly to ", local); return Optional.of(local); } diff --git a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java index 4878691742c..5de0c5eff74 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java @@ -1,8 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.cluster; - -import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.search.result.ErrorMessage; @@ -43,7 +41,7 @@ public class ClusterMonitor<T> { public ClusterMonitor(NodeManager<T> manager, String ignored) { this(manager); } - + public ClusterMonitor(NodeManager<T> manager) { nodeManager = manager; monitorThread = new MonitorThread("search.clustermonitor"); diff --git a/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java index c1c955cb03a..ea881ad8b48 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.cluster; +import com.yahoo.container.protect.Error; import com.yahoo.search.result.ErrorMessage; /** @@ -36,20 +37,14 @@ public class TrafficNodeMonitor<T> extends BaseNodeMonitor<T> { public void failed(ErrorMessage error) { respondedAt = now(); - switch (error.getCode()) { - // TODO: Remove hard coded error messages. - // Refer to docs/errormessages - case 10: - case 11: - // Only count not being able to talk to backend at all - // as errors we care about - if ((respondedAt-succeededAt) > 10000) { - setWorking(false, "Not working for 10 s: " + error.toString()); - } - break; - default: - succeededAt = respondedAt; - break; + if (error.getCode() == Error.BACKEND_COMMUNICATION_ERROR.code) { + setWorking(false, "Connection failure: " + error.toString()); + } else if (error.getCode() == Error.NO_ANSWER_WHEN_PINGING_NODE.code) { + if ((respondedAt - succeededAt) > 10000) { + setWorking(false, "Not working for 10 s: " + error.toString()); + } + } else { + succeededAt = respondedAt; } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index 286eee004c5..0dd682dee0e 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -11,6 +11,9 @@ import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.SearchPath.InvalidSearchPathException; +import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.vespa.config.search.DispatchConfig; import java.util.Arrays; @@ -33,6 +36,8 @@ import java.util.Set; * @author ollvir */ public class Dispatcher extends AbstractComponent { + private static final int MAX_GROUP_SELECTION_ATTEMPTS = 3; + /** If enabled, this internal dispatcher will be preferred over fdispatch whenever possible */ private static final CompoundName dispatchInternal = new CompoundName("dispatch.internal"); @@ -66,11 +71,6 @@ public class Dispatcher extends AbstractComponent { rpcResourcePool.release(); } - @FunctionalInterface - private interface SearchInvokerSupplier { - Optional<SearchInvoker> supply(Query query, List<SearchCluster.Node> nodes); - } - public Optional<FillInvoker> getFillInvoker(Result result, VespaBackEndSearcher searcher, DocumentDatabase documentDb, FS4InvokerFactory fs4InvokerFactory) { Optional<FillInvoker> rpcInvoker = rpcResourcePool.getFillInvoker(result.getQuery(), searcher, documentDb); @@ -102,6 +102,11 @@ public class Dispatcher extends AbstractComponent { return Optional.empty(); } + @FunctionalInterface + private interface SearchInvokerSupplier { + Optional<SearchInvoker> supply(Query query, int groupId, List<Node> nodes, boolean acceptIncompleteCoverage); + } + // build invoker based on searchpath private Optional<SearchInvoker> getSearchPathInvoker(Query query, SearchInvokerSupplier invokerFactory) { String searchPath = query.getModel().getSearchPath(); @@ -109,11 +114,11 @@ public class Dispatcher extends AbstractComponent { return Optional.empty(); } try { - List<SearchCluster.Node> nodes = SearchPath.selectNodes(searchPath, searchCluster); + List<Node> nodes = SearchPath.selectNodes(searchPath, searchCluster); if (nodes.isEmpty()) { return Optional.empty(); } else { - return invokerFactory.supply(query, nodes); + return invokerFactory.supply(query, -1, nodes, true); } } catch (InvalidSearchPathException e) { return Optional.of(new SearchErrorInvoker(e.getMessage())); @@ -121,41 +126,34 @@ public class Dispatcher extends AbstractComponent { } private Optional<SearchInvoker> getInternalInvoker(Query query, SearchInvokerSupplier invokerFactory) { - Optional<SearchCluster.Node> directNode = searchCluster.directDispatchTarget(); + Optional<Node> directNode = searchCluster.directDispatchTarget(); if (directNode.isPresent()) { - SearchCluster.Node node = directNode.get(); + Node node = directNode.get(); query.trace(false, 2, "Dispatching directly to ", node); - return invokerFactory.supply(query, Arrays.asList(node)); + return invokerFactory.supply(query, -1, Arrays.asList(node), true); } - Set<Integer> tried = null; - int max = searchCluster.groups().size(); - for (int attempt = 0; attempt < max; attempt++) { - Optional<SearchCluster.Group> groupInCluster = loadBalancer.takeGroupForQuery(query); - if (! groupInCluster.isPresent()) { + int max = Integer.min(searchCluster.orderedGroups().size(), MAX_GROUP_SELECTION_ATTEMPTS); + Set<Integer> rejected = null; + for (int i = 0; i < max; i++) { + Optional<Group> groupInCluster = loadBalancer.takeGroupForQuery(query, rejected); + if (!groupInCluster.isPresent()) { // No groups available break; } - SearchCluster.Group group = groupInCluster.get(); - if (tried != null && tried.contains(group.id())) { - // bail out: LB is offering a previously discarded group - loadBalancer.releaseGroup(group); - break; - } - - Optional<SearchInvoker> invoker = invokerFactory.supply(query, group.nodes()); + Group group = groupInCluster.get(); + boolean acceptIncompleteCoverage = (i == max - 1); + Optional<SearchInvoker> invoker = invokerFactory.supply(query, group.id(), group.nodes(), acceptIncompleteCoverage); if (invoker.isPresent()) { query.trace(false, 2, "Dispatching internally to ", group); invoker.get().teardown(() -> loadBalancer.releaseGroup(group)); return invoker; } else { - // invoker could not be produced (likely connectivity issue) - searchCluster.groupConnectionFailure(group); loadBalancer.releaseGroup(group); - if (tried == null) { - tried = new HashSet<>(); + if (rejected == null) { + rejected = new HashSet<>(); } - tried.add(group.id()); + rejected.add(group.id()); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java index 64e38a488ab..22573fac2d9 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java @@ -2,12 +2,14 @@ package com.yahoo.search.dispatch; import com.yahoo.search.Query; -import com.yahoo.search.dispatch.SearchCluster.Group; +import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -47,18 +49,19 @@ public class LoadBalancer { * {@link #releaseGroup} symmetrically for each taken allocation. * * @param query the query for which this allocation is made + * @param rejectedGroups if not null, the load balancer will only return groups with IDs not in the set * @return The node group to target, or <i>empty</i> if the internal dispatch logic cannot be used */ - public Optional<Group> takeGroupForQuery(Query query) { + public Optional<Group> takeGroupForQuery(Query query, Set<Integer> rejectedGroups) { if (scoreboard == null) { return Optional.empty(); } - return allocateNextGroup(); + return allocateNextGroup(rejectedGroups); } /** - * Release an allocation given by {@link #takeGroupForQuery(Query)}. The release must be done exactly once for each allocation. + * Release an allocation given by {@link #takeGroupForQuery}. The release must be done exactly once for each allocation. * * @param group * previously allocated group @@ -74,7 +77,7 @@ public class LoadBalancer { } } - private Optional<Group> allocateNextGroup() { + private Optional<Group> allocateNextGroup(Set<Integer> rejectedGroups) { synchronized (this) { GroupSchedule bestSchedule = null; int bestIndex = needle; @@ -82,9 +85,11 @@ public class LoadBalancer { int index = needle; for (int i = 0; i < scoreboard.size(); i++) { GroupSchedule sched = scoreboard.get(index); - if (sched.isPreferredOver(bestSchedule)) { - bestSchedule = sched; - bestIndex = index; + if (rejectedGroups == null || !rejectedGroups.contains(sched.group.id())) { + if (sched.isPreferredOver(bestSchedule)) { + bestSchedule = sched; + bestIndex = index; + } } index = nextScoreboardIndex(index); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java index 57f06225d27..6800a80b78f 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java @@ -3,7 +3,9 @@ package com.yahoo.search.dispatch; import com.google.common.collect.ImmutableCollection; import com.yahoo.collections.Pair; -import com.yahoo.search.dispatch.SearchCluster.Group; +import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import java.util.ArrayList; import java.util.Collection; @@ -37,7 +39,7 @@ public class SearchPath { * @return list of nodes chosen with the search path, or an empty list in which * case some other node selection logic should be used */ - public static List<SearchCluster.Node> selectNodes(String searchPath, SearchCluster cluster) { + public static List<Node> selectNodes(String searchPath, SearchCluster cluster) { Optional<SearchPath> sp = SearchPath.fromString(searchPath); if (sp.isPresent()) { return sp.get().mapToNodes(cluster); @@ -46,7 +48,7 @@ public class SearchPath { } } - public static Optional<SearchPath> fromString(String path) { + static Optional<SearchPath> fromString(String path) { if (path == null || path.isEmpty()) { return Optional.empty(); } @@ -73,23 +75,23 @@ public class SearchPath { this.group = group; } - private List<SearchCluster.Node> mapToNodes(SearchCluster cluster) { + private List<Node> mapToNodes(SearchCluster cluster) { if (cluster.groups().isEmpty()) { return Collections.emptyList(); } - SearchCluster.Group selectedGroup = selectGroup(cluster); + Group selectedGroup = selectGroup(cluster); if (nodes.isEmpty()) { return selectedGroup.nodes(); } - List<SearchCluster.Node> groupNodes = selectedGroup.nodes(); + List<Node> groupNodes = selectedGroup.nodes(); Set<Integer> wanted = new HashSet<>(); int max = groupNodes.size(); for (NodeSelection node : nodes) { wanted.addAll(node.matches(max)); } - List<SearchCluster.Node> ret = new ArrayList<>(); + List<Node> ret = new ArrayList<>(); for (int idx : wanted) { ret.add(groupNodes.get(idx)); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java new file mode 100644 index 00000000000..01cbc5cd307 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java @@ -0,0 +1,75 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch.searchcluster; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; + +/** + * A group in a search cluster. This class is multithread safe. + * + * @author bratseth + * @author ollivir + */ +public class Group { + + private final int id; + private final ImmutableList<Node> nodes; + + private final AtomicBoolean hasSufficientCoverage = new AtomicBoolean(true); + private final AtomicLong activeDocuments = new AtomicLong(0); + + public Group(int id, List<Node> nodes) { + this.id = id; + this.nodes = ImmutableList.copyOf(nodes); + } + + /** Returns the unique identity of this group */ + public int id() { return id; } + + /** Returns the nodes in this group as an immutable list */ + public ImmutableList<Node> nodes() { return nodes; } + + /** + * Returns whether this group has sufficient active documents + * (compared to other groups) that is should receive traffic + */ + public boolean hasSufficientCoverage() { + return hasSufficientCoverage.get(); + } + + void setHasSufficientCoverage(boolean sufficientCoverage) { + hasSufficientCoverage.lazySet(sufficientCoverage); + } + + void aggregateActiveDocuments() { + long activeDocumentsInGroup = 0; + for (Node node : nodes) { + if (node.isWorking()) { + activeDocumentsInGroup += node.getActiveDocuments(); + } + } + activeDocuments.set(activeDocumentsInGroup); + + } + + /** Returns the active documents on this node. If unknown, 0 is returned. */ + long getActiveDocuments() { + return this.activeDocuments.get(); + } + + @Override + public String toString() { return "search group " + id; } + + @Override + public int hashCode() { return id; } + + @Override + public boolean equals(Object other) { + if (other == this) return true; + if (!(other instanceof Group)) return false; + return ((Group) other).id == this.id; + } +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java new file mode 100644 index 00000000000..98deb9e3199 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java @@ -0,0 +1,73 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch.searchcluster; + +import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; + +/** + * A node in a search cluster. This class is multithread safe. + * + * @author bratseth + * @author ollivir + */ +public class Node { + + private final int key; + private final String hostname; + private final int fs4port; + final int group; + + private final AtomicBoolean working = new AtomicBoolean(true); + private final AtomicLong activeDocuments = new AtomicLong(0); + + public Node(int key, String hostname, int fs4port, int group) { + this.key = key; + this.hostname = hostname; + this.fs4port = fs4port; + this.group = group; + } + + /** Returns the unique and stable distribution key of this node */ + public int key() { return key; } + + public String hostname() { return hostname; } + + public int fs4port() { return fs4port; } + + /** Returns the id of this group this node belongs to */ + public int group() { return group; } + + public void setWorking(boolean working) { + this.working.lazySet(working); + } + + /** Returns whether this node is currently responding to requests */ + public boolean isWorking() { return working.get(); } + + /** Updates the active documents on this node */ + void setActiveDocuments(long activeDocuments) { + this.activeDocuments.set(activeDocuments); + } + + /** Returns the active documents on this node. If unknown, 0 is returned. */ + public long getActiveDocuments() { + return this.activeDocuments.get(); + } + + @Override + public int hashCode() { return Objects.hash(hostname, fs4port); } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if ( ! (o instanceof Node)) return false; + Node other = (Node)o; + if ( ! Objects.equals(this.hostname, other.hostname)) return false; + if ( ! Objects.equals(this.fs4port, other.fs4port)) return false; + return true; + } + + @Override + public String toString() { return "search node " + hostname + ":" + fs4port + " in group " + group; } +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Pinger.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Pinger.java new file mode 100644 index 00000000000..7c7a9cb1d1c --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Pinger.java @@ -0,0 +1,42 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch.searchcluster; + +import com.yahoo.prelude.Ping; +import com.yahoo.prelude.Pong; +import com.yahoo.prelude.fastsearch.FS4ResourcePool; +import com.yahoo.prelude.fastsearch.FastSearcher; +import com.yahoo.search.cluster.ClusterMonitor; +import com.yahoo.search.result.ErrorMessage; +import com.yahoo.yolean.Exceptions; + +import java.util.concurrent.Callable; + +/** + * @author bratseth + * @author ollivir + */ +class Pinger implements Callable<Pong> { + private final Node node; + private final ClusterMonitor<Node> clusterMonitor; + private final FS4ResourcePool fs4ResourcePool; + + public Pinger(Node node, ClusterMonitor<Node> clusterMonitor, FS4ResourcePool fs4ResourcePool) { + this.node = node; + this.clusterMonitor = clusterMonitor; + this.fs4ResourcePool = fs4ResourcePool; + } + + public Pong call() { + try { + Pong pong = FastSearcher.ping(new Ping(clusterMonitor.getConfiguration().getRequestTimeout()), + fs4ResourcePool.getBackend(node.hostname(), node.fs4port()), node.toString()); + if (pong.activeDocuments().isPresent()) + node.setActiveDocuments(pong.activeDocuments().get()); + return pong; + } catch (RuntimeException e) { + return new Pong(ErrorMessage.createBackendCommunicationError("Exception when pinging " + node + ": " + + Exceptions.toMessageString(e))); + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java index e26dd5648eb..b0e63d20931 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java @@ -1,5 +1,5 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.search.dispatch; +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch.searchcluster; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; @@ -11,27 +11,18 @@ import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.cluster.NodeManager; import com.yahoo.search.result.ErrorMessage; import com.yahoo.vespa.config.search.DispatchConfig; - -// Only needed until query requests are moved to rpc -import com.yahoo.prelude.Ping; -import com.yahoo.prelude.fastsearch.FastSearcher; -import com.yahoo.yolean.Exceptions; import com.yahoo.prelude.Pong; import com.yahoo.prelude.fastsearch.FS4ResourcePool; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; -import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.FutureTask; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -41,7 +32,7 @@ import java.util.stream.Collectors; * * @author bratseth */ -public class SearchCluster implements NodeManager<SearchCluster.Node> { +public class SearchCluster implements NodeManager<Node> { private static final Logger log = Logger.getLogger(SearchCluster.class.getName()); @@ -128,8 +119,8 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> { // Only use direct dispatch if we have exactly 1 search node on the same machine: if (localSearchNodes.size() != 1) return Optional.empty(); - SearchCluster.Node localSearchNode = localSearchNodes.iterator().next(); - SearchCluster.Group localSearchGroup = groups.get(localSearchNode.group()); + Node localSearchNode = localSearchNodes.iterator().next(); + Group localSearchGroup = groups.get(localSearchNode.group()); // Only use direct dispatch if the local search node has the entire corpus if (localSearchGroup.nodes().size() != 1) return Optional.empty(); @@ -188,7 +179,7 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> { if ( ! directDispatchTarget.isPresent()) return Optional.empty(); // Only use direct dispatch if the local group has sufficient coverage - SearchCluster.Group localSearchGroup = groups.get(directDispatchTarget.get().group()); + Group localSearchGroup = groups.get(directDispatchTarget.get().group()); if ( ! localSearchGroup.hasSufficientCoverage()) return Optional.empty(); // Only use direct dispatch if the local search node is up @@ -216,10 +207,6 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> { vipStatus.removeFromRotation(this); } - public void groupConnectionFailure(Group group) { - group.setHasSufficientCoverage(false); // will be reset after next ping iteration - } - private void updateSufficientCoverage(Group group, boolean sufficientCoverage) { // update VIP status if we direct dispatch to this group and coverage status changed if (usesDirectDispatchTo(group) && sufficientCoverage != group.hasSufficientCoverage()) { @@ -245,7 +232,7 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> { /** Used by the cluster monitor to manage node status */ @Override public void ping(Node node, Executor executor) { - Pinger pinger = new Pinger(node); + Pinger pinger = new Pinger(node, clusterMonitor, fs4ResourcePool); FutureTask<Pong> futurePong = new FutureTask<>(pinger); executor.execute(futurePong); Pong pong = getPong(futurePong, node); @@ -287,29 +274,34 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> { Group group = orderedGroups.get(i); long activeDocuments = activeDocumentsInGroup[i]; long averageDocumentsInOtherGroups = (sumOfActiveDocuments - activeDocuments) / (numGroups - 1); - boolean sufficientCoverage = true; - - if (averageDocumentsInOtherGroups > 0) { - double coverage = 100.0 * (double) activeDocuments / averageDocumentsInOtherGroups; - sufficientCoverage = coverage >= minActivedocsCoveragePercentage; - } - if (sufficientCoverage) { - sufficientCoverage = isNodeCoverageSufficient(group); - } + boolean sufficientCoverage = isGroupCoverageSufficient(group.nodes(), activeDocuments, averageDocumentsInOtherGroups); updateSufficientCoverage(group, sufficientCoverage); } } - private boolean isNodeCoverageSufficient(Group group) { + private boolean isGroupCoverageSufficient(List<Node> nodes, long activeDocuments, long averageDocumentsInOtherGroups) { + boolean sufficientCoverage = true; + + if (averageDocumentsInOtherGroups > 0) { + double coverage = 100.0 * (double) activeDocuments / averageDocumentsInOtherGroups; + sufficientCoverage = coverage >= minActivedocsCoveragePercentage; + } + if (sufficientCoverage) { + sufficientCoverage = isGroupNodeCoverageSufficient(nodes); + } + return sufficientCoverage; + } + + private boolean isGroupNodeCoverageSufficient(List<Node> nodes) { int nodesUp = 0; - for (Node node : group.nodes()) { + for (Node node : nodes) { if (node.isWorking()) { nodesUp++; } } - int nodes = group.nodes().size(); - int nodesAllowedDown = maxNodesDownPerGroup + (int) (((double) nodes * (100.0 - minGroupCoverage)) / 100.0); - return nodesUp + nodesAllowedDown >= nodes; + int numNodes = nodes.size(); + int nodesAllowedDown = maxNodesDownPerGroup + (int) (((double) numNodes * (100.0 - minGroupCoverage)) / 100.0); + return nodesUp + nodesAllowedDown >= numNodes; } private Pong getPong(FutureTask<Pong> futurePong, Node node) { @@ -326,153 +318,26 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> { } } - private class Pinger implements Callable<Pong> { - - private final Node node; - - public Pinger(Node node) { - this.node = node; - } - - public Pong call() { - try { - Pong pong = FastSearcher.ping(new Ping(clusterMonitor.getConfiguration().getRequestTimeout()), - fs4ResourcePool.getBackend(node.hostname(), node.fs4port()), node.toString()); - if (pong.activeDocuments().isPresent()) - node.setActiveDocuments(pong.activeDocuments().get()); - return pong; - } catch (RuntimeException e) { - return new Pong(ErrorMessage.createBackendCommunicationError("Exception when pinging " + node + ": " - + Exceptions.toMessageString(e))); - } - } - - } - - /** A group in a search cluster. This class is multithread safe. */ - public static class Group { - - private final int id; - private final ImmutableList<Node> nodes; - - private final AtomicBoolean hasSufficientCoverage = new AtomicBoolean(true); - private final AtomicLong activeDocuments = new AtomicLong(0); - - public Group(int id, List<Node> nodes) { - this.id = id; - this.nodes = ImmutableList.copyOf(nodes); - } - - /** Returns the unique identity of this group */ - public int id() { return id; } - - /** Returns the nodes in this group as an immutable list */ - public ImmutableList<Node> nodes() { return nodes; } - - /** - * Returns whether this group has sufficient active documents - * (compared to other groups) that is should receive traffic - */ - public boolean hasSufficientCoverage() { - return hasSufficientCoverage.get(); - } - - void setHasSufficientCoverage(boolean sufficientCoverage) { - hasSufficientCoverage.lazySet(sufficientCoverage); + /** + * Calculate whether a subset of nodes in a group has enough coverage + */ + public boolean isPartialGroupCoverageSufficient(int groupId, List<Node> nodes) { + if (orderedGroups.size() == 1) { + return true; } - - void aggregateActiveDocuments() { - long activeDocumentsInGroup = 0; - for (Node node : nodes) { - if (node.isWorking()) { - activeDocumentsInGroup += node.getActiveDocuments(); - } + long sumOfActiveDocuments = 0; + int otherGroups = 0; + for (Group g : orderedGroups) { + if (g.id() != groupId) { + sumOfActiveDocuments += g.getActiveDocuments(); + otherGroups++; } - activeDocuments.set(activeDocumentsInGroup); - } - - /** Returns the active documents on this node. If unknown, 0 is returned. */ - long getActiveDocuments() { - return this.activeDocuments.get(); + long activeDocuments = 0; + for (Node n : nodes) { + activeDocuments += n.getActiveDocuments(); } - - @Override - public String toString() { return "search group " + id; } - - @Override - public int hashCode() { return id; } - - @Override - public boolean equals(Object other) { - if (other == this) return true; - if (!(other instanceof Group)) return false; - return ((Group) other).id == this.id; - } - + long averageDocumentsInOtherGroups = sumOfActiveDocuments / otherGroups; + return isGroupCoverageSufficient(nodes, activeDocuments, averageDocumentsInOtherGroups); } - - /** A node in a search cluster. This class is multithread safe. */ - public static class Node { - - private final int key; - private final String hostname; - private final int fs4port; - private final int group; - - private final AtomicBoolean working = new AtomicBoolean(true); - private final AtomicLong activeDocuments = new AtomicLong(0); - - public Node(int key, String hostname, int fs4port, int group) { - this.key = key; - this.hostname = hostname; - this.fs4port = fs4port; - this.group = group; - } - - /** Returns the unique and stable distribution key of this node */ - public int key() { return key; } - - public String hostname() { return hostname; } - - public int fs4port() { return fs4port; } - - /** Returns the id of this group this node belongs to */ - public int group() { return group; } - - void setWorking(boolean working) { - this.working.lazySet(working); - } - - /** Returns whether this node is currently responding to requests */ - public boolean isWorking() { return working.get(); } - - /** Updates the active documents on this node */ - void setActiveDocuments(long activeDocuments) { - this.activeDocuments.set(activeDocuments); - } - - /** Returns the active documents on this node. If unknown, 0 is returned. */ - public long getActiveDocuments() { - return this.activeDocuments.get(); - } - - @Override - public int hashCode() { return Objects.hash(hostname, fs4port); } - - @Override - public boolean equals(Object o) { - if (o == this) return true; - if ( ! (o instanceof Node)) return false; - Node other = (Node)o; - if ( ! Objects.equals(this.hostname, other.hostname)) return false; - if ( ! Objects.equals(this.fs4port, other.fs4port)) return false; - return true; - } - - @Override - public String toString() { return "search node " + hostname + ":" + fs4port + " in group " + group; } - - } - } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java index b08aef6ecb1..79b43563c6a 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java @@ -5,26 +5,37 @@ import com.google.common.collect.ImmutableList; import com.yahoo.component.chain.Chain; import com.yahoo.config.subscription.ConfigGetter; import com.yahoo.container.handler.VipStatus; +import com.yahoo.container.protect.Error; import com.yahoo.container.search.Fs4Config; import com.yahoo.document.GlobalId; -import com.yahoo.fs4.mplex.*; +import com.yahoo.fs4.BasicPacket; +import com.yahoo.fs4.Packet; +import com.yahoo.fs4.QueryPacket; +import com.yahoo.fs4.mplex.Backend; +import com.yahoo.fs4.mplex.BackendTestCase; import com.yahoo.fs4.test.QueryTestCase; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.prelude.Ping; import com.yahoo.prelude.Pong; +import com.yahoo.prelude.fastsearch.CacheControl; +import com.yahoo.prelude.fastsearch.CacheKey; +import com.yahoo.prelude.fastsearch.CacheParams; +import com.yahoo.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; -import com.yahoo.container.protect.Error; -import com.yahoo.fs4.*; +import com.yahoo.prelude.fastsearch.FS4ResourcePool; +import com.yahoo.prelude.fastsearch.FastHit; +import com.yahoo.prelude.fastsearch.FastSearcher; +import com.yahoo.prelude.fastsearch.PacketWrapper; +import com.yahoo.prelude.fastsearch.SummaryParameters; import com.yahoo.prelude.fastsearch.test.fs4mock.MockBackend; import com.yahoo.prelude.fastsearch.test.fs4mock.MockFS4ResourcePool; import com.yahoo.prelude.fastsearch.test.fs4mock.MockFSChannel; +import com.yahoo.prelude.query.WordItem; import com.yahoo.processing.execution.Execution.Trace; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; -import com.yahoo.prelude.fastsearch.*; -import com.yahoo.prelude.query.WordItem; -import com.yahoo.search.dispatch.SearchCluster; +import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.grouping.GroupingRequest; import com.yahoo.search.grouping.request.AllOperation; import com.yahoo.search.grouping.request.EachOperation; @@ -48,9 +59,13 @@ import java.util.logging.Level; import java.util.logging.Logger; import static org.hamcrest.CoreMatchers.containsString; -import static org.junit.Assert.*; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * Tests the Fast searcher @@ -87,7 +102,7 @@ public class FastSearcherTestCase { @Test public void testNullQuery() { Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL); - FastSearcher fastSearcher = new FastSearcher(new MockBackend(), + FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool(1), new MockDispatcher(Collections.emptyList()), new SummaryParameters(null), @@ -115,9 +130,9 @@ public class FastSearcherTestCase { .rankprofile(new DocumentdbInfoConfig.Documentdb.Rankprofile.Builder() .name("simpler").hasRankFeatures(false).hasSummaryFeatures(false)))); - List<SearchCluster.Node> nodes = new ArrayList<>(); - nodes.add(new SearchCluster.Node(0, "host1", 5000, 0)); - nodes.add(new SearchCluster.Node(2, "host2", 5000, 0)); + List<Node> nodes = new ArrayList<>(); + nodes.add(new Node(0, "host1", 5000, 0)); + nodes.add(new Node(2, "host2", 5000, 0)); MockFS4ResourcePool mockFs4ResourcePool = new MockFS4ResourcePool(); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), @@ -162,15 +177,15 @@ public class FastSearcherTestCase { new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("testDb"))); FastSearcher fastSearcher = new FastSearcher(mockBackend, new FS4ResourcePool(1), - new MockDispatcher(Collections.emptyList()), + new MockDispatcher(Collections.emptyList()), new SummaryParameters(null), new ClusterParams("testhittype"), - new CacheParams(100, 1e64), + new CacheParams(100, 1e64), documentdbConfigWithOneDb); Query query = new Query("?query=foo&model.restrict=testDb"); query.prepare(); - Result result = doSearch(fastSearcher, query, 0, 10); + doSearch(fastSearcher, query, 0, 10); Packet receivedPacket = mockBackend.getChannel().getLastQueryPacket(); byte[] encoded = QueryTestCase.packetToBytes(receivedPacket); @@ -354,10 +369,10 @@ public class FastSearcherTestCase { Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL); return new FastSearcher(mockBackend, new FS4ResourcePool(1), - new MockDispatcher(Collections.emptyList()), + new MockDispatcher(Collections.emptyList()), new SummaryParameters(null), - new ClusterParams("testhittype"), - new CacheParams(100, 1e64), + new ClusterParams("testhittype"), + new CacheParams(100, 1e64), config); } @@ -436,12 +451,12 @@ public class FastSearcherTestCase { } } - + @Test public void testSinglePassGroupingIsForcedWithSingleNodeGroups() { FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool(1), - new MockDispatcher(new SearchCluster.Node(0, "host0", 123, 0)), + new MockDispatcher(new Node(0, "host0", 123, 0)), new SummaryParameters(null), new ClusterParams("testhittype"), new CacheParams(100, 1e64), @@ -455,17 +470,17 @@ public class FastSearcherTestCase { all.addChild(new EachOperation()); all.addChild(new EachOperation()); request2.setRootOperation(all); - + assertForceSinglePassIs(false, q); fastSearcher.search(q, new Execution(Execution.Context.createContextStub())); - assertForceSinglePassIs(true, q); + assertForceSinglePassIs(true, q); } @Test public void testSinglePassGroupingIsNotForcedWithSingleNodeGroups() { - MockDispatcher dispatcher = - new MockDispatcher(ImmutableList.of(new SearchCluster.Node(0, "host0", 123, 0), - new SearchCluster.Node(2, "host1", 123, 0))); + MockDispatcher dispatcher = + new MockDispatcher(ImmutableList.of(new Node(0, "host0", 123, 0), + new Node(2, "host1", 123, 0))); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new FS4ResourcePool(1), @@ -495,7 +510,7 @@ public class FastSearcherTestCase { } private void assertForceSinglePassIs(boolean expected, GroupingOperation operation) { - assertEquals("Force single pass is " + expected + " in " + operation, + assertEquals("Force single pass is " + expected + " in " + operation, expected, operation.getForceSinglePass()); for (GroupingOperation child : operation.getChildren()) assertForceSinglePassIs(expected, child); diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java index 4f6d2d88917..12c313dbfe3 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java @@ -12,16 +12,14 @@ import com.yahoo.prelude.fastsearch.FastSearcher; import com.yahoo.prelude.fastsearch.SummaryParameters; import com.yahoo.prelude.fastsearch.test.fs4mock.MockBackend; import com.yahoo.prelude.fastsearch.test.fs4mock.MockFS4ResourcePool; -import com.yahoo.prelude.fastsearch.test.fs4mock.MockFSChannel; import com.yahoo.search.Query; import com.yahoo.search.Result; -import com.yahoo.search.dispatch.SearchCluster; +import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.searchchain.Execution; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -38,7 +36,7 @@ class FastSearcherTester { private final MockDispatcher mockDispatcher; private final VipStatus vipStatus; - public FastSearcherTester(int containerClusterSize, SearchCluster.Node searchNode) { + public FastSearcherTester(int containerClusterSize, Node searchNode) { this(containerClusterSize, Collections.singletonList(searchNode)); } @@ -46,7 +44,7 @@ class FastSearcherTester { this(containerClusterSize, toNodes(hostAndPortAndGroupStrings)); } - public FastSearcherTester(int containerClusterSize, List<SearchCluster.Node> searchNodes) { + public FastSearcherTester(int containerClusterSize, List<Node> searchNodes) { ClustersStatus clustersStatus = new ClustersStatus(); clustersStatus.setContainerHasClusters(true); vipStatus = new VipStatus(clustersStatus); @@ -61,12 +59,12 @@ class FastSearcherTester { new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder())); } - private static List<SearchCluster.Node> toNodes(String... hostAndPortAndGroupStrings) { - List<SearchCluster.Node> nodes = new ArrayList<>(); + private static List<Node> toNodes(String... hostAndPortAndGroupStrings) { + List<Node> nodes = new ArrayList<>(); int key = 0; for (String s : hostAndPortAndGroupStrings) { String[] parts = s.split(":"); - nodes.add(new SearchCluster.Node(key++, parts[0], Integer.parseInt(parts[1]), Integer.parseInt(parts[2]))); + nodes.add(new Node(key++, parts[0], Integer.parseInt(parts[1]), Integer.parseInt(parts[2]))); } return nodes; } @@ -90,7 +88,7 @@ class FastSearcherTester { mockFS4ResourcePool.setResponding(hostname, responding); // Make the search cluster monitor notice right now in this thread - SearchCluster.Node node = mockDispatcher.searchCluster().nodesByHost().get(hostname).iterator().next(); + Node node = mockDispatcher.searchCluster().nodesByHost().get(hostname).iterator().next(); mockDispatcher.searchCluster().ping(node, MoreExecutors.directExecutor()); } @@ -99,7 +97,7 @@ class FastSearcherTester { mockFS4ResourcePool.setActiveDocuments(hostname, activeDocuments); // Make the search cluster monitor notice right now in this thread - SearchCluster.Node node = mockDispatcher.searchCluster().nodesByHost().get(hostname).iterator().next(); + Node node = mockDispatcher.searchCluster().nodesByHost().get(hostname).iterator().next(); mockDispatcher.searchCluster().ping(node, MoreExecutors.directExecutor()); mockDispatcher.searchCluster().pingIterationCompleted(); } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java index 2b25b2a3796..800b1bc21f0 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java @@ -5,7 +5,7 @@ import com.yahoo.container.handler.VipStatus; import com.yahoo.prelude.fastsearch.FS4ResourcePool; import com.yahoo.search.Result; import com.yahoo.search.dispatch.Dispatcher; -import com.yahoo.search.dispatch.SearchCluster; +import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.vespa.config.search.DispatchConfig; import java.util.Collections; @@ -13,27 +13,27 @@ import java.util.List; class MockDispatcher extends Dispatcher { - public MockDispatcher(SearchCluster.Node node) { + public MockDispatcher(Node node) { this(Collections.singletonList(node)); } - public MockDispatcher(List<SearchCluster.Node> nodes) { + public MockDispatcher(List<Node> nodes) { super(toDispatchConfig(nodes), new FS4ResourcePool(1), 1, new VipStatus()); } - public MockDispatcher(List<SearchCluster.Node> nodes, VipStatus vipStatus) { + public MockDispatcher(List<Node> nodes, VipStatus vipStatus) { super(toDispatchConfig(nodes), new FS4ResourcePool(1), 1, vipStatus); } - public MockDispatcher(List<SearchCluster.Node> nodes, FS4ResourcePool fs4ResourcePool, + public MockDispatcher(List<Node> nodes, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) { super(toDispatchConfig(nodes), fs4ResourcePool, containerClusterSize, vipStatus); } - private static DispatchConfig toDispatchConfig(List<SearchCluster.Node> nodes) { + private static DispatchConfig toDispatchConfig(List<Node> nodes) { DispatchConfig.Builder dispatchConfigBuilder = new DispatchConfig.Builder(); int key = 0; - for (SearchCluster.Node node : nodes) { + for (Node node : nodes) { DispatchConfig.Node.Builder dispatchConfigNodeBuilder = new DispatchConfig.Node.Builder(); dispatchConfigNodeBuilder.host(node.hostname()); dispatchConfigNodeBuilder.fs4port(node.fs4port()); @@ -44,7 +44,7 @@ class MockDispatcher extends Dispatcher { } return new DispatchConfig(dispatchConfigBuilder); } - + public void fill(Result result, String summaryClass) { } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java index 9311ddab3c6..698cee743e4 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java @@ -2,8 +2,9 @@ package com.yahoo.search.dispatch; import com.yahoo.search.Query; -import com.yahoo.search.dispatch.SearchCluster.Group; -import com.yahoo.search.dispatch.SearchCluster.Node; +import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import junit.framework.AssertionFailedError; import org.junit.Test; @@ -21,11 +22,11 @@ import static org.junit.Assert.assertThat; public class LoadBalancerTest { @Test public void requreThatLoadBalancerServesSingleNodeSetups() { - Node n1 = new SearchCluster.Node(0, "test-node1", 0, 0); + Node n1 = new Node(0, "test-node1", 0, 0); SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1), null, 1, null); LoadBalancer lb = new LoadBalancer(cluster, true); - Optional<Group> grp = lb.takeGroupForQuery(new Query()); + Optional<Group> grp = lb.takeGroupForQuery(new Query(), null); Group group = grp.orElseGet(() -> { throw new AssertionFailedError("Expected a SearchCluster.Group"); }); @@ -34,12 +35,12 @@ public class LoadBalancerTest { @Test public void requreThatLoadBalancerServesMultiGroupSetups() { - Node n1 = new SearchCluster.Node(0, "test-node1", 0, 0); - Node n2 = new SearchCluster.Node(1, "test-node2", 1, 1); + Node n1 = new Node(0, "test-node1", 0, 0); + Node n2 = new Node(1, "test-node2", 1, 1); SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null); LoadBalancer lb = new LoadBalancer(cluster, true); - Optional<Group> grp = lb.takeGroupForQuery(new Query()); + Optional<Group> grp = lb.takeGroupForQuery(new Query(), null); Group group = grp.orElseGet(() -> { throw new AssertionFailedError("Expected a SearchCluster.Group"); }); @@ -48,51 +49,51 @@ public class LoadBalancerTest { @Test public void requreThatLoadBalancerServesClusteredGroups() { - Node n1 = new SearchCluster.Node(0, "test-node1", 0, 0); - Node n2 = new SearchCluster.Node(1, "test-node2", 1, 0); - Node n3 = new SearchCluster.Node(0, "test-node3", 0, 1); - Node n4 = new SearchCluster.Node(1, "test-node4", 1, 1); + Node n1 = new Node(0, "test-node1", 0, 0); + Node n2 = new Node(1, "test-node2", 1, 0); + Node n3 = new Node(0, "test-node3", 0, 1); + Node n4 = new Node(1, "test-node4", 1, 1); SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2, n3, n4), null, 2, null); LoadBalancer lb = new LoadBalancer(cluster, true); - Optional<Group> grp = lb.takeGroupForQuery(new Query()); + Optional<Group> grp = lb.takeGroupForQuery(new Query(), null); assertThat(grp.isPresent(), is(true)); } @Test public void requreThatLoadBalancerReturnsDifferentGroups() { - Node n1 = new SearchCluster.Node(0, "test-node1", 0, 0); - Node n2 = new SearchCluster.Node(1, "test-node2", 1, 1); + Node n1 = new Node(0, "test-node1", 0, 0); + Node n2 = new Node(1, "test-node2", 1, 1); SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null); LoadBalancer lb = new LoadBalancer(cluster, true); // get first group - Optional<Group> grp = lb.takeGroupForQuery(new Query()); + Optional<Group> grp = lb.takeGroupForQuery(new Query(), null); Group group = grp.get(); int id1 = group.id(); // release allocation lb.releaseGroup(group); // get second group - grp = lb.takeGroupForQuery(new Query()); + grp = lb.takeGroupForQuery(new Query(), null); group = grp.get(); assertThat(group.id(), not(equalTo(id1))); } @Test public void requreThatLoadBalancerReturnsGroupWithShortestQueue() { - Node n1 = new SearchCluster.Node(0, "test-node1", 0, 0); - Node n2 = new SearchCluster.Node(1, "test-node2", 1, 1); + Node n1 = new Node(0, "test-node1", 0, 0); + Node n2 = new Node(1, "test-node2", 1, 1); SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null); LoadBalancer lb = new LoadBalancer(cluster, true); // get first group - Optional<Group> grp = lb.takeGroupForQuery(new Query()); + Optional<Group> grp = lb.takeGroupForQuery(new Query(), null); Group group = grp.get(); int id1 = group.id(); // get second group - grp = lb.takeGroupForQuery(new Query()); + grp = lb.takeGroupForQuery(new Query(), null); group = grp.get(); int id2 = group.id(); assertThat(id2, not(equalTo(id1))); @@ -100,7 +101,7 @@ public class LoadBalancerTest { lb.releaseGroup(group); // get third group - grp = lb.takeGroupForQuery(new Query()); + grp = lb.takeGroupForQuery(new Query(), null); group = grp.get(); assertThat(group.id(), equalTo(id2)); } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java index ee903fd3fa0..0c0a65ded17 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java @@ -3,6 +3,9 @@ package com.yahoo.search.dispatch; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; +import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.SearchCluster; import java.util.ArrayList; import java.util.Collections; diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java index 77cb8d5c353..a1f926d3201 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java @@ -2,6 +2,7 @@ package com.yahoo.search.dispatch; import com.yahoo.search.dispatch.SearchPath.InvalidSearchPathException; +import com.yahoo.search.dispatch.searchcluster.Node; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -77,7 +78,7 @@ public class SearchPathTest { assertThat(distKeysAsString(SearchPath.selectNodes("[1,88>/1", cluster)), equalTo("5,6")); } - private static String distKeysAsString(Collection<SearchCluster.Node> nodes) { - return nodes.stream().map(SearchCluster.Node::key).map(Object::toString).collect(Collectors.joining(",")); + private static String distKeysAsString(Collection<Node> nodes) { + return nodes.stream().map(Node::key).map(Object::toString).collect(Collectors.joining(",")); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java index 44f29eb6113..ac080fe15d7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java @@ -3,26 +3,17 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; +import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.ClusterUtilization; import com.yahoo.vespa.hosted.controller.application.Deployment; -import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; import java.time.Duration; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.logging.Level; /** * Fetch utilization metrics and update applications with this data. @@ -32,12 +23,10 @@ import java.util.logging.Level; public class ClusterUtilizationMaintainer extends Maintainer { private final Controller controller; - private final List<String> baseUris; - public ClusterUtilizationMaintainer(Controller controller, Duration duration, JobControl jobControl, ApiAuthorityConfig apiAuthorityConfig) { + public ClusterUtilizationMaintainer(Controller controller, Duration duration, JobControl jobControl) { super(controller, duration, jobControl); this.controller = controller; - this.baseUris = apiAuthorityConfig.authorities(); } private Map<ClusterSpec.Id, ClusterUtilization> getUpdatedClusterUtilizations(ApplicationId app, ZoneId zone) { @@ -55,45 +44,15 @@ public class ClusterUtilizationMaintainer extends Maintainer { @Override protected void maintain() { - try (CloseableHttpClient httpClient = HttpClientBuilder.create().build()) { - String uri = baseUris.get(0) + "metricforwarding/v1/clusterutilization"; // For now, we only feed to one controller - Slime slime = getMetricSlime(); - ByteArrayEntity entity = new ByteArrayEntity(SlimeUtils.toJsonBytes(slime)); - HttpPost httpPost = new HttpPost(uri); - httpPost.setEntity(entity); - httpClient.execute(httpPost); - } catch (Exception e) { - log.log(Level.WARNING, "Failed to update cluster utilization metrics", e); - } - - } - - private Slime getMetricSlime() { - Slime slime = new Slime(); - Cursor cursor = slime.setArray(); for (Application application : controller().applications().asList()) { - Cursor applicationCursor = cursor.addObject(); - applicationCursor.setString("applicationId", application.id().serializedForm()); - Cursor deploymentArray = applicationCursor.setArray("deployments"); for (Deployment deployment : application.deployments().values()) { - Cursor deploymentEntry = deploymentArray.addObject(); - deploymentEntry.setString("zoneId", deployment.zone().value()); - Cursor clusterArray = deploymentEntry.setArray("clusterUtil"); + Map<ClusterSpec.Id, ClusterUtilization> clusterUtilization = getUpdatedClusterUtilizations(application.id(), deployment.zone()); - fillClusterUtilization(clusterArray, clusterUtilization); + + controller().applications().lockIfPresent(application.id(), lockedApplication -> + controller().applications().store(lockedApplication.withClusterUtilization(deployment.zone(), clusterUtilization))); } } - return slime; } - private void fillClusterUtilization(Cursor cursor, Map<ClusterSpec.Id, ClusterUtilization> clusterUtilization) { - for (Map.Entry<ClusterSpec.Id, ClusterUtilization> entry : clusterUtilization.entrySet()) { - Cursor clusterUtilCursor = cursor.addObject(); - clusterUtilCursor.setString("clusterSpecId", entry.getKey().value()); - clusterUtilCursor.setDouble("cpu", entry.getValue().getCpu()); - clusterUtilCursor.setDouble("memory", entry.getValue().getMemory()); - clusterUtilCursor.setDouble("disk", entry.getValue().getDisk()); - clusterUtilCursor.setDouble("diskBusy", entry.getValue().getDiskBusy()); - } - } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java index 3cf3c47a6f3..bf5743e2d3c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java @@ -1,31 +1,17 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; -import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; import com.yahoo.config.provision.SystemName; import com.yahoo.log.LogLevel; -import com.yahoo.slime.ArrayTraverser; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Inspector; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Organization; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; +import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.yolean.Exceptions; -import org.apache.http.HttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.util.EntityUtils; -import java.io.IOException; import java.time.Duration; -import java.util.ArrayList; import java.util.EnumSet; import java.util.List; import java.util.Objects; @@ -43,107 +29,44 @@ public class ContactInformationMaintainer extends Maintainer { private static final Logger log = Logger.getLogger(ContactInformationMaintainer.class.getName()); private final Organization organization; - private final List<String> baseUris; - private CloseableHttpClient httpClient = HttpClientBuilder.create().build(); - public ContactInformationMaintainer(Controller controller, Duration interval, JobControl jobControl, Organization organization, ApiAuthorityConfig apiAuthorityConfig) { + public ContactInformationMaintainer(Controller controller, Duration interval, JobControl jobControl, Organization organization) { super(controller, interval, jobControl, null, EnumSet.of(SystemName.cd, SystemName.main)); this.organization = Objects.requireNonNull(organization, "organization must be non-null"); - this.baseUris = apiAuthorityConfig.authorities(); } - // The maintainer will eventually feed contact info to systems other than its own, determined by the baseUris list. @Override protected void maintain() { - for (String baseUri : baseUris) { - for (String tenantName : getTenantList(baseUri)) { - Optional<PropertyId> tenantPropertyId = getPropertyId(tenantName, baseUri); - if (!tenantPropertyId.isPresent()) - continue; - findContact(tenantPropertyId.get()).ifPresent(contact -> { - feedContact(tenantName, contact, baseUri); + for (Tenant t : controller().tenants().asList()) { + if (!(t instanceof AthenzTenant)) continue; // No contact information for non-Athenz tenants + AthenzTenant tenant = (AthenzTenant) t; + if (!tenant.propertyId().isPresent()) continue; // Can only update contact information if property ID is known + try { + findContact(tenant).ifPresent(contact -> { + controller().tenants().lockIfPresent(t.name(), lockedTenant -> controller().tenants().store(lockedTenant.with(contact))); }); + } catch (Exception e) { + log.log(LogLevel.WARNING, "Failed to update contact information for " + tenant + ": " + + Exceptions.toMessageString(e) + ". Retrying in " + + maintenanceInterval()); } } } - private void feedContact(String tenantName, Contact contact, String baseUri) { - try { - CloseableHttpClient httpClient = HttpClientBuilder.create().build(); - String uri = baseUri + "contactinfo/v1/tenant/" + tenantName; - HttpPost httpPost = new HttpPost(uri); - httpPost.setEntity(contactToByteArrayEntity(contact)); - httpClient.execute(httpPost); - } catch (Exception e) { - log.log(LogLevel.WARNING, "Failed to update contact information for " + tenantName + ": " + - Exceptions.toMessageString(e) + ". Retrying in " + - maintenanceInterval()); - } - } - - private ByteArrayEntity contactToByteArrayEntity(Contact contact) throws IOException { - Slime slime = new Slime(); - Cursor cursor = slime.setObject(); - cursor.setString("url", contact.url().toString()); - cursor.setString("issueTrackerUrl", contact.issueTrackerUrl().toString()); - cursor.setString("propertyUrl", contact.propertyUrl().toString()); - Cursor personsCursor = cursor.setArray("persons"); - for (List<String> personList : contact.persons()) { - Cursor sublist = personsCursor.addArray(); - for(String person : personList) { - sublist.addString(person); - } - } - return new ByteArrayEntity(SlimeUtils.toJsonBytes(slime)); - } - - private List<String> getTenantList(String baseUri) { - List<String> tenantList = new ArrayList<>(); - HttpGet getRequest = new HttpGet(baseUri + "application/v4/tenant/"); - try { - HttpResponse response = httpClient.execute(getRequest); - Slime slime = SlimeUtils.jsonToSlime(EntityUtils.toByteArray(response.getEntity())); - Inspector inspector = slime.get(); - inspector.traverse((ArrayTraverser) (index, tenant) -> { - tenantList.add(tenant.field("tenant").asString()); - }); - } catch (IOException e) { - log.log(LogLevel.WARNING, "Failed to get tenant list from base URI: " + baseUri.toString() + - Exceptions.toMessageString(e) + ". Retrying in " + - maintenanceInterval()); - } - return tenantList; - } - - private Optional<PropertyId> getPropertyId(String tenantName, String baseUri) { - Optional<PropertyId> propertyId = Optional.empty(); - HttpGet getRequest = new HttpGet(baseUri + "application/v4/tenant/" + tenantName); - try { - HttpResponse response = httpClient.execute(getRequest); - Slime slime = SlimeUtils.jsonToSlime(EntityUtils.toByteArray(response.getEntity())); - Inspector inspector = slime.get(); - if (!inspector.field("propertyId").valid()) { - log.log(LogLevel.WARNING, "Unable to get property id for " + tenantName); - return Optional.empty(); - } - propertyId = Optional.of(new PropertyId(inspector.field("propertyId").asString())); - } catch (IOException e) { - log.log(LogLevel.WARNING, "Unable to get property idfor " + tenantName, e); - } - return propertyId; - } - /** Find contact information for given tenant */ - private Optional<Contact> findContact(PropertyId propertyId) { - List<List<String>> persons = organization.contactsFor(propertyId) + private Optional<Contact> findContact(AthenzTenant tenant) { + if (!tenant.propertyId().isPresent()) { + return Optional.empty(); + } + List<List<String>> persons = organization.contactsFor(tenant.propertyId().get()) .stream() .map(personList -> personList.stream() .map(User::displayName) .collect(Collectors.toList())) .collect(Collectors.toList()); - return Optional.of(new Contact(organization.contactsUri(propertyId), - organization.propertyUri(propertyId), - organization.issueCreationUri(propertyId), + return Optional.of(new Contact(organization.contactsUri(tenant.propertyId().get()), + organization.propertyUri(tenant.propertyId().get()), + organization.issueCreationUri(tenant.propertyId().get()), persons)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index b2968270af9..7b17f38bd78 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -65,15 +65,15 @@ public class ControllerMaintenance extends AbstractComponent { upgrader = new Upgrader(controller, maintenanceInterval, jobControl, curator); readyJobsTrigger = new ReadyJobsTrigger(controller, Duration.ofMinutes(1), jobControl); clusterInfoMaintainer = new ClusterInfoMaintainer(controller, Duration.ofHours(2), jobControl, nodeRepositoryClient); - clusterUtilizationMaintainer = new ClusterUtilizationMaintainer(controller, Duration.ofHours(2), jobControl, apiAuthorityConfig); - deploymentMetricsMaintainer = new DeploymentMetricsMaintainer(controller, Duration.ofMinutes(10), jobControl, apiAuthorityConfig); + clusterUtilizationMaintainer = new ClusterUtilizationMaintainer(controller, Duration.ofHours(2), jobControl); + deploymentMetricsMaintainer = new DeploymentMetricsMaintainer(controller, Duration.ofMinutes(10), jobControl); applicationOwnershipConfirmer = new ApplicationOwnershipConfirmer(controller, Duration.ofHours(12), jobControl, ownershipIssues); dnsMaintainer = new DnsMaintainer(controller, Duration.ofHours(12), jobControl, nameService); systemUpgrader = new SystemUpgrader(controller, Duration.ofMinutes(1), jobControl); jobRunner = new JobRunner(controller, Duration.ofMinutes(2), jobControl); osUpgraders = osUpgraders(controller, jobControl); osVersionStatusUpdater = new OsVersionStatusUpdater(controller, maintenanceInterval, jobControl); - contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl, organization, apiAuthorityConfig); + contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl, organization); } public Upgrader upgrader() { return upgrader; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index 22d738ff98c..c17d070e92e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -2,27 +2,18 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.HostName; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.RotationStatus; -import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; -import org.apache.http.HttpResponse; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; +import com.yahoo.yolean.Exceptions; -import java.io.IOException; import java.time.Duration; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -47,12 +38,10 @@ public class DeploymentMetricsMaintainer extends Maintainer { private static final int applicationsToUpdateInParallel = 10; private final ApplicationController applications; - private final List<String> baseUris; - public DeploymentMetricsMaintainer(Controller controller, Duration duration, JobControl jobControl, ApiAuthorityConfig apiAuthorityConfig) { + public DeploymentMetricsMaintainer(Controller controller, Duration duration, JobControl jobControl) { super(controller, duration, jobControl); this.applications = controller.applications(); - baseUris = apiAuthorityConfig.authorities(); } @Override @@ -63,27 +52,27 @@ public class DeploymentMetricsMaintainer extends Maintainer { // Run parallel stream inside a custom ForkJoinPool so that we can control the number of threads used ForkJoinPool pool = new ForkJoinPool(applicationsToUpdateInParallel); - Slime slime = new Slime(); - Cursor cursor = slime.setArray(); pool.submit(() -> { applicationList.parallelStream().forEach(application -> { try { - MetricsService.ApplicationMetrics applicationMetrics = controller().metricsService() - .getApplicationMetrics(application.id()); - Map<HostName, RotationStatus> rotationStatus = getRotationStatus(application); - Map<ZoneId, MetricsService.DeploymentMetrics> deploymentMetrics = getDeploymentMetrics(application); - - - Cursor applicationCursor = cursor.addObject(); - applicationCursor.setString("applicationId", application.id().serializedForm()); - Cursor applicationMetricsCursor = applicationCursor.setObject("applicationMetrics"); - fillApplicationMetrics(applicationMetricsCursor, applicationMetrics); - Cursor rotationStatusCursor = applicationCursor.setArray("rotationStatus"); - fillRotationStatus(rotationStatusCursor, rotationStatus); - Cursor deploymentArray = applicationCursor.setArray("deploymentMetrics"); - for (Map.Entry<ZoneId, MetricsService.DeploymentMetrics> entry : deploymentMetrics.entrySet()) { - Cursor deploymentEntry = deploymentArray.addObject(); - fillDeploymentMetrics(deploymentEntry, entry.getKey().value(), entry.getValue()); + applications.lockIfPresent(application.id(), locked -> + applications.store(locked.with(controller().metricsService().getApplicationMetrics(application.id())))); + + applications.lockIfPresent(application.id(), locked -> + applications.store(locked.withRotationStatus(rotationStatus(application)))); + + for (Deployment deployment : application.deployments().values()) { + MetricsService.DeploymentMetrics deploymentMetrics = controller().metricsService() + .getDeploymentMetrics(application.id(), deployment.zone()); + DeploymentMetrics newMetrics = new DeploymentMetrics(deploymentMetrics.queriesPerSecond(), + deploymentMetrics.writesPerSecond(), + deploymentMetrics.documentCount(), + deploymentMetrics.queryLatencyMillis(), + deploymentMetrics.writeLatencyMillis()); + + applications.lockIfPresent(application.id(), locked -> + applications.store(locked.with(deployment.zone(), newMetrics) + .recordActivityAt(controller().clock().instant(), deployment.zone()))); } } catch (Exception e) { failures.incrementAndGet(); @@ -91,26 +80,23 @@ public class DeploymentMetricsMaintainer extends Maintainer { } }); }); - pool.shutdown(); try { pool.awaitTermination(30, TimeUnit.MINUTES); - feedMetrics(slime); if (lastException.get() != null) { - log.log(Level.WARNING, String.format("Failed to query metrics service for %d/%d applications. Retrying in %s. Stacktrace of last error: ", + log.log(Level.WARNING, String.format("Failed to query metrics service for %d/%d applications. Last error: %s. Retrying in %s", failures.get(), applicationList.size(), - maintenanceInterval()),lastException.get()); + Exceptions.toMessageString(lastException.get()), + maintenanceInterval())); } } catch (InterruptedException e) { throw new RuntimeException(e); - } catch (IOException e) { - log.log(Level.WARNING, "Unable to feed metrics to API", e); } } /** Get global rotation status for application */ - private Map<HostName, RotationStatus> getRotationStatus(Application application) { + private Map<HostName, RotationStatus> rotationStatus(Application application) { return applications.rotationRepository().getRotation(application) .map(rotation -> controller().metricsService().getRotationStatus(rotation.name())) .map(rotationStatus -> { @@ -121,50 +107,6 @@ public class DeploymentMetricsMaintainer extends Maintainer { .orElseGet(Collections::emptyMap); } - private Map<ZoneId, MetricsService.DeploymentMetrics> getDeploymentMetrics(Application application) { - Map<ZoneId, MetricsService.DeploymentMetrics> deploymentMetrics = new HashMap<>(); - for (Deployment deployment : application.deployments().values()) { - ZoneId zone = deployment.zone(); - MetricsService.DeploymentMetrics zoneDeploymentMetrics = controller().metricsService() - .getDeploymentMetrics(application.id(), zone); - deploymentMetrics.put(zone, zoneDeploymentMetrics); - } - return deploymentMetrics; - } - - private void fillApplicationMetrics(Cursor applicationCursor, MetricsService.ApplicationMetrics applicationMetrics) { - applicationCursor.setDouble("queryServiceQuality", applicationMetrics.queryServiceQuality()); - applicationCursor.setDouble("writeServiceQuality", applicationMetrics.writeServiceQuality()); - } - - private void fillRotationStatus(Cursor rotationStatusCursor, Map<HostName, RotationStatus> rotationStatus) { - for (Map.Entry<HostName, RotationStatus> entry : rotationStatus.entrySet()) { - Cursor rotationStatusEntry = rotationStatusCursor.addObject(); - rotationStatusEntry.setString("hostname", entry.getKey().value()); - rotationStatusEntry.setString("rotationStatus", entry.getValue().toString()); - } - } - - private void fillDeploymentMetrics(Cursor deploymentCursor, String zone, MetricsService.DeploymentMetrics deploymentMetrics) { - deploymentCursor.setString("zoneId", zone); - deploymentCursor.setDouble("queriesPerSecond", deploymentMetrics.queriesPerSecond()); - deploymentCursor.setDouble("writesPerSecond", deploymentMetrics.writesPerSecond()); - deploymentCursor.setDouble("documentCount", deploymentMetrics.documentCount()); - deploymentCursor.setDouble("queryLatencyMillis", deploymentMetrics.queryLatencyMillis()); - deploymentCursor.setDouble("writeLatencyMillis", deploymentMetrics.writeLatencyMillis()); - } - - private void feedMetrics(Slime slime) throws IOException { - String uri = baseUris.get(0) + "/metricforwarding/v1/deploymentmetrics"; // For now, we only feed to one controller - CloseableHttpClient httpClient = HttpClientBuilder.create().build(); - HttpPost httpPost = new HttpPost(uri); - httpPost.setEntity(new ByteArrayEntity(SlimeUtils.toJsonBytes(slime))); - HttpResponse response = httpClient.execute(httpPost); - if (response.getStatusLine().getStatusCode() != 200) { - log.log(Level.WARNING, "Could not feed metrics. Reason: " + response.getStatusLine().getReasonPhrase()); - } - } - private static RotationStatus from(com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus status) { switch (status) { case IN: return RotationStatus.in; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 34014f24862..fa5d713d6b3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -203,6 +203,8 @@ class JobControllerApiHandlerHelper { private static void jobTypeToSlime(Cursor jobObject, Controller controller, Application application, JobType type, DeploymentSteps steps, Map<JobType, Versions> pendingProduction, Map<JobType, Run> running, URI baseUriForJob) { + application.deploymentJobs().statusOf(type).ifPresent(status -> status.pausedUntil().ifPresent(until -> + jobObject.setLong("pausedUntil", until))); int runs = 0; Cursor runArray = jobObject.setArray("runs"); if (type.isTest()) { @@ -239,7 +241,9 @@ class JobControllerApiHandlerHelper { runObject.setString("status", "pending"); versionsToSlime(runObject, pendingProduction.get(type)); Cursor pendingObject = runObject.setObject("tasks"); - if ( ! controller.applications().deploymentTrigger().triggerAt(controller.clock().instant(), type, versions, application)) + if (application.deploymentJobs().statusOf(type).map(status -> status.pausedUntil().isPresent()).orElse(false)) + pendingObject.setString("paused", "pending"); + else if ( ! controller.applications().deploymentTrigger().triggerAt(controller.clock().instant(), type, versions, application)) pendingObject.setString("cooldown", "failed"); else { int pending = 0; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java index ccd943e7f0a..b1e3f8799d6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java @@ -81,10 +81,10 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { @Override public Optional<ErrorResponse> filterRequest(DiscFilterRequest request) { Method method = getMethod(request); - Path path = new Path(request.getRequestURI()); - if (isWhiteListed(method, path)) return Optional.empty(); + if (isWhiteListedMethod(method)) return Optional.empty(); try { + Path path = new Path(request.getRequestURI()); AthenzPrincipal principal = getPrincipalOrThrow(request); if (isWhiteListedOperation(path, method)) { // no authz check @@ -106,10 +106,8 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { } } - private static boolean isWhiteListed(Method method, Path path) { - return WHITELISTED_METHODS.contains(method) || - path.matches("/metricforwarding/v1/{*}") && method == POST || - path.matches("/contactinfo/v1/{*}") && method == POST; + private static boolean isWhiteListedMethod(Method method) { + return WHITELISTED_METHODS.contains(method); } private static boolean isWhiteListedOperation(Path path, Method method) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainerTest.java index 71d2690bb4a..66be2d09797 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainerTest.java @@ -3,45 +3,24 @@ package com.yahoo.vespa.hosted.controller.maintenance; // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -import com.github.tomakehurst.wiremock.junit.WireMockRule; -import com.github.tomakehurst.wiremock.stubbing.ServeEvent; -import com.github.tomakehurst.wiremock.verification.LoggedRequest; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.application.Deployment; -import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import org.junit.Assert; -import org.junit.Rule; import org.junit.Test; import java.time.Duration; -import java.util.List; - -import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; -import static com.github.tomakehurst.wiremock.client.WireMock.findAll; -import static com.github.tomakehurst.wiremock.client.WireMock.get; -import static com.github.tomakehurst.wiremock.client.WireMock.getAllServeEvents; -import static com.github.tomakehurst.wiremock.client.WireMock.okJson; -import static com.github.tomakehurst.wiremock.client.WireMock.post; -import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; -import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; -import static org.junit.Assert.assertEquals; /** * @author smorgrav */ public class ClusterUtilizationMaintainerTest { - @Rule - public WireMockRule wireMockRule = new WireMockRule(4443); - @Test public void maintain() { - wireMockRule.stubFor(post(urlEqualTo("/metricforwarding/v1/clusterutilization")) - .willReturn(aResponse().withStatus(200))); ControllerTester tester = new ControllerTester(); ApplicationId app = tester.createAndDeploy("tenant1", "domain1", "app1", Environment.dev, 123).id(); @@ -49,16 +28,12 @@ public class ClusterUtilizationMaintainerTest { Deployment deployment = tester.controller().applications().get(app).get().deployments().values().stream().findAny().get(); Assert.assertEquals(0, deployment.clusterUtils().size()); - ApiAuthorityConfig.Builder apiAuthorityConfigBuilder = new ApiAuthorityConfig.Builder().authorities("http://localhost:4443/"); - ApiAuthorityConfig apiAuthorityConfig = new ApiAuthorityConfig(apiAuthorityConfigBuilder); - ClusterUtilizationMaintainer maintainer = new ClusterUtilizationMaintainer(tester.controller(), Duration.ofHours(1), new JobControl(new MockCuratorDb()), apiAuthorityConfig); - maintainer.maintain(); + ClusterUtilizationMaintainer mainainer = new ClusterUtilizationMaintainer(tester.controller(), Duration.ofHours(1), new JobControl(new MockCuratorDb())); + mainainer.maintain(); - List<ServeEvent> allServeEvents = getAllServeEvents(); - assertEquals(allServeEvents.size(), 1); - LoggedRequest request = findAll(postRequestedFor(urlEqualTo("/metricforwarding/v1/clusterutilization"))).get(0); - String expectedBody = "[{\"applicationId\":\"tenant1:app1:default\",\"deployments\":[{\"zoneId\":\"dev.us-east-1\",\"clusterUtil\":[{\"clusterSpecId\":\"default\",\"cpu\":0.5554,\"memory\":0.6990000000000001,\"disk\":0.34590000000000004,\"diskBusy\":0.0}]}]}]"; - assertEquals(expectedBody, new String(request.getBody())); + deployment = tester.controller().applications().get(app).get().deployments().values().stream().findAny().get(); + Assert.assertEquals(1, deployment.clusterUtils().size()); + Assert.assertEquals(0.5554, deployment.clusterUtils().get(ClusterSpec.Id.from("default")).getCpu(), Double.MIN_VALUE); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java index a46215eaf22..cbaa37b15e3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java @@ -1,17 +1,13 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; -import com.github.tomakehurst.wiremock.junit.WireMockRule; -import com.github.tomakehurst.wiremock.verification.LoggedRequest; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.ControllerTester; -import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Contact; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import java.net.URI; @@ -22,16 +18,9 @@ import java.util.List; import java.util.function.Supplier; import java.util.stream.Collectors; -import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; -import static com.github.tomakehurst.wiremock.client.WireMock.findAll; -import static com.github.tomakehurst.wiremock.client.WireMock.get; -import static com.github.tomakehurst.wiremock.client.WireMock.okJson; -import static com.github.tomakehurst.wiremock.client.WireMock.post; -import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; -import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; -import static com.github.tomakehurst.wiremock.client.WireMock.verify; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * @author mpolden @@ -40,27 +29,13 @@ public class ContactInformationMaintainerTest { private ControllerTester tester; private ContactInformationMaintainer maintainer; - private String contactInfoPath = "/contactinfo/v1/tenant/tenant1"; - private String tenantPath = "/application/v4/tenant/"; - - @Rule - public WireMockRule wireMockRule = new WireMockRule(4443); - @Before public void before() { tester = new ControllerTester(); - ApiAuthorityConfig.Builder apiAuthorityConfigBuilder = new ApiAuthorityConfig.Builder().authorities("http://localhost:4443/"); - ApiAuthorityConfig apiAuthorityConfig = new ApiAuthorityConfig(apiAuthorityConfigBuilder); maintainer = new ContactInformationMaintainer(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()), - tester.organization(), apiAuthorityConfig); - wireMockRule.stubFor(post(urlEqualTo(contactInfoPath)) - .willReturn(aResponse().withStatus(200))); - wireMockRule.stubFor(get(urlEqualTo(tenantPath)) - .willReturn(okJson("[{\"tenant\":\"tenant1\"}]"))); - wireMockRule.stubFor(get(urlEqualTo(tenantPath + "tenant1")) - .willReturn(okJson("{\"tenant\":\"tenant1\", \"athensDomain\":\"domain\", \"property\":\"property\", \"propertyId\":\"1\"}"))); + tester.organization()); } @Test @@ -69,13 +44,13 @@ public class ContactInformationMaintainerTest { TenantName name = tester.createTenant("tenant1", "domain1", propertyId); Supplier<AthenzTenant> tenant = () -> tester.controller().tenants().requireAthenzTenant(name); assertFalse("No contact information initially", tenant.get().contact().isPresent()); + Contact contact = testContact(); registerContact(propertyId, contact); maintainer.run(); - verify(1, postRequestedFor(urlEqualTo(contactInfoPath))); - LoggedRequest request = findAll(postRequestedFor(urlEqualTo(contactInfoPath))).get(0); - String expectedBody = "{\"url\":\"http://contact1.test\",\"issueTrackerUrl\":\"http://issue-tracker1.test\",\"propertyUrl\":\"http://property1.test\",\"persons\":[[\"alice\"],[\"bob\"]]}"; - assertEquals(expectedBody, new String(request.getBody())); + + assertTrue("Contact information added", tenant.get().contact().isPresent()); + assertEquals(contact, tenant.get().contact().get()); } private void registerContact(long propertyId, Contact contact) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java index fc43a7f9411..e11440a372c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java @@ -1,38 +1,27 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; -import com.github.tomakehurst.wiremock.junit.WireMockRule; -import com.github.tomakehurst.wiremock.stubbing.ServeEvent; -import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; -import com.yahoo.slime.ArrayTraverser; -import com.yahoo.slime.Inspector; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; +import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.application.RotationStatus; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.MetricsServiceMock; -import org.junit.Rule; import org.junit.Test; import java.time.Duration; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; -import static com.github.tomakehurst.wiremock.client.WireMock.findAll; -import static com.github.tomakehurst.wiremock.client.WireMock.getAllServeEvents; -import static com.github.tomakehurst.wiremock.client.WireMock.post; -import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor; -import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; -import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import java.time.Instant; +import java.util.function.Supplier; + +import static java.time.temporal.ChronoUnit.MILLIS; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; /** * @author smorgrav @@ -40,18 +29,69 @@ import static org.junit.Assert.assertEquals; */ public class DeploymentMetricsMaintainerTest { - private static final double DELTA = 0.0000001; - - @Rule - public WireMockRule wireMockRule = new WireMockRule(options().dynamicPort(), true); + @Test + public void updates_metrics() { + ControllerTester tester = new ControllerTester(); + ApplicationId appId = tester.createAndDeploy("tenant1", "domain1", "app1", + Environment.dev, 123).id(); + DeploymentMetricsMaintainer maintainer = maintainer(tester.controller()); + Supplier<Application> app = tester.application(appId); + Supplier<Deployment> deployment = () -> app.get().deployments().values().stream().findFirst().get(); + + // No metrics gathered yet + assertEquals(0, app.get().metrics().queryServiceQuality(), 0); + assertEquals(0, deployment.get().metrics().documentCount(), 0); + assertFalse("Never received any queries", deployment.get().activity().lastQueried().isPresent()); + assertFalse("Never received any writes", deployment.get().activity().lastWritten().isPresent()); + + // Metrics are gathered and saved to application + maintainer.maintain(); + assertEquals(0.5, app.get().metrics().queryServiceQuality(), Double.MIN_VALUE); + assertEquals(0.7, app.get().metrics().writeServiceQuality(), Double.MIN_VALUE); + assertEquals(1, deployment.get().metrics().queriesPerSecond(), Double.MIN_VALUE); + assertEquals(2, deployment.get().metrics().writesPerSecond(), Double.MIN_VALUE); + assertEquals(3, deployment.get().metrics().documentCount(), Double.MIN_VALUE); + assertEquals(4, deployment.get().metrics().queryLatencyMillis(), Double.MIN_VALUE); + assertEquals(5, deployment.get().metrics().writeLatencyMillis(), Double.MIN_VALUE); + Instant t1 = tester.clock().instant().truncatedTo(MILLIS); + assertEquals(t1, deployment.get().activity().lastQueried().get()); + assertEquals(t1, deployment.get().activity().lastWritten().get()); + + // Time passes. Activity is updated as app is still receiving traffic + tester.clock().advance(Duration.ofHours(1)); + Instant t2 = tester.clock().instant().truncatedTo(MILLIS); + maintainer.maintain(); + assertEquals(t2, deployment.get().activity().lastQueried().get()); + assertEquals(t2, deployment.get().activity().lastWritten().get()); + assertEquals(1, deployment.get().activity().lastQueriesPerSecond().getAsDouble(), Double.MIN_VALUE); + assertEquals(2, deployment.get().activity().lastWritesPerSecond().getAsDouble(), Double.MIN_VALUE); + + // Query traffic disappears. Query activity stops updating + tester.clock().advance(Duration.ofHours(1)); + Instant t3 = tester.clock().instant().truncatedTo(MILLIS); + tester.metricsService().setMetric("queriesPerSecond", 0D); + tester.metricsService().setMetric("writesPerSecond", 5D); + maintainer.maintain(); + assertEquals(t2, deployment.get().activity().lastQueried().get()); + assertEquals(t3, deployment.get().activity().lastWritten().get()); + assertEquals(1, deployment.get().activity().lastQueriesPerSecond().getAsDouble(), Double.MIN_VALUE); + assertEquals(5, deployment.get().activity().lastWritesPerSecond().getAsDouble(), Double.MIN_VALUE); + + // Feed traffic disappears. Feed activity stops updating + tester.clock().advance(Duration.ofHours(1)); + tester.metricsService().setMetric("writesPerSecond", 0D); + maintainer.maintain(); + assertEquals(t2, deployment.get().activity().lastQueried().get()); + assertEquals(t3, deployment.get().activity().lastWritten().get()); + assertEquals(1, deployment.get().activity().lastQueriesPerSecond().getAsDouble(), Double.MIN_VALUE); + assertEquals(5, deployment.get().activity().lastWritesPerSecond().getAsDouble(), Double.MIN_VALUE); + } @Test - public void maintain() { + public void updates_rotation_status() { DeploymentTester tester = new DeploymentTester(); MetricsServiceMock metricsService = tester.controllerTester().metricsService(); - ApiAuthorityConfig.Builder apiAuthorityConfigBuilder = new ApiAuthorityConfig.Builder().authorities("http://localhost:" + wireMockRule.port() + "/"); - ApiAuthorityConfig apiAuthorityConfig = new ApiAuthorityConfig(apiAuthorityConfigBuilder); - DeploymentMetricsMaintainer maintainer = new DeploymentMetricsMaintainer(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()), apiAuthorityConfig); + DeploymentMetricsMaintainer maintainer = maintainer(tester.controller()); Application application = tester.createApplication("app1", "tenant1", 1, 1L); ZoneId zone1 = ZoneId.from("prod", "us-west-1"); ZoneId zone2 = ZoneId.from("prod", "us-east-3"); @@ -65,68 +105,32 @@ public class DeploymentMetricsMaintainerTest { .build(); tester.deployCompletely(application, applicationPackage); + Supplier<Application> app = () -> tester.application(application.id()); + Supplier<Deployment> deployment1 = () -> app.get().deployments().get(zone1); + Supplier<Deployment> deployment2 = () -> app.get().deployments().get(zone2); String assignedRotation = "rotation-fqdn-01"; tester.controllerTester().metricsService().addRotation(assignedRotation); + // No status gathered yet + assertEquals(RotationStatus.unknown, app.get().rotationStatus(deployment1.get())); + assertEquals(RotationStatus.unknown, app.get().rotationStatus(deployment2.get())); + // One rotation out, one in metricsService.setZoneIn(assignedRotation, "proxy.prod.us-west-1.vip.test"); metricsService.setZoneOut(assignedRotation,"proxy.prod.us-east-3.vip.test"); - - wireMockRule.stubFor(post(urlEqualTo("/metricforwarding/v1/deploymentmetrics")) - .willReturn(aResponse().withStatus(200))); maintainer.maintain(); + assertEquals(RotationStatus.in, app.get().rotationStatus(deployment1.get())); + assertEquals(RotationStatus.out, app.get().rotationStatus(deployment2.get())); - List<ServeEvent> allServeEvents = getAllServeEvents(); - assertEquals(1, allServeEvents.size()); - LoggedRequest request = findAll(postRequestedFor(urlEqualTo("/metricforwarding/v1/deploymentmetrics"))).get(0); - - Slime slime = SlimeUtils.jsonToSlime(request.getBody()); - Inspector inspector = slime.get().entry(0); - assertEquals("tenant1:app1:default", inspector.field("applicationId").asString()); - MetricsService.ApplicationMetrics applicationMetrics = applicationMetricsFromInspector(inspector.field("applicationMetrics")); - assertEquals(0.5, applicationMetrics.queryServiceQuality(), DELTA); - assertEquals(0.7, applicationMetrics.writeServiceQuality(), DELTA); - - Map<String, String> rotationStatus = rotationsStatusFromInspector(inspector.field("rotationStatus")); - assertEquals("in", rotationStatus.get("proxy.prod.us-west-1.vip.test")); - assertEquals("out", rotationStatus.get("proxy.prod.us-east-3.vip.test")); - - Map<String, MetricsService.DeploymentMetrics> deploymentMetricsByZone = deploymentMetricsFromInspector(inspector.field("deploymentMetrics")); - MetricsService.DeploymentMetrics deploymentMetrics = deploymentMetricsByZone.get("prod.us-west-1"); - assertEquals(1.0, deploymentMetrics.queriesPerSecond(), DELTA); - assertEquals(2.0, deploymentMetrics.writesPerSecond(), DELTA); - assertEquals(3.0, deploymentMetrics.documentCount(), DELTA); - assertEquals(4.0, deploymentMetrics.queryLatencyMillis(), DELTA); - assertEquals(5.0, deploymentMetrics.writeLatencyMillis(), DELTA); - - deploymentMetrics = deploymentMetricsByZone.get("prod.us-east-3"); - assertEquals(1.0, deploymentMetrics.queriesPerSecond(), DELTA); - assertEquals(2.0, deploymentMetrics.writesPerSecond(), DELTA); - assertEquals(3.0, deploymentMetrics.documentCount(), DELTA); - assertEquals(4.0, deploymentMetrics.queryLatencyMillis(), DELTA); - assertEquals(5.0, deploymentMetrics.writeLatencyMillis(), DELTA); - } - - private MetricsService.ApplicationMetrics applicationMetricsFromInspector(Inspector inspector) { - return new MetricsService.ApplicationMetrics(inspector.field("queryServiceQuality").asDouble(), inspector.field("writeServiceQuality").asDouble()); + // All rotations in + metricsService.setZoneIn(assignedRotation,"proxy.prod.us-east-3.vip.test"); + maintainer.maintain(); + assertEquals(RotationStatus.in, app.get().rotationStatus(deployment1.get())); + assertEquals(RotationStatus.in, app.get().rotationStatus(deployment2.get())); } - private Map<String, String> rotationsStatusFromInspector(Inspector inspector) { - HashMap<String, String> rotationStatus = new HashMap<>(); - inspector.traverse((ArrayTraverser) (index, entry) -> { - rotationStatus.put(entry.field("hostname").asString(), entry.field("rotationStatus").asString()); - }); - return rotationStatus; + private static DeploymentMetricsMaintainer maintainer(Controller controller) { + return new DeploymentMetricsMaintainer(controller, Duration.ofDays(1), new JobControl(controller.curator())); } - private Map<String, MetricsService.DeploymentMetrics> deploymentMetricsFromInspector(Inspector inspector) { - Map<String, MetricsService.DeploymentMetrics> deploymentMetricByZone = new HashMap<>(); - inspector.traverse((ArrayTraverser) (index, entry) -> { - String zone = entry.field("zoneId").asString(); - MetricsService.DeploymentMetrics deploymentMetrics = new MetricsService.DeploymentMetrics(entry.field("queriesPerSecond").asDouble(), entry.field("writesPerSecond").asDouble(), - entry.field("documentCount").asLong(), entry.field("queryLatencyMillis").asDouble(), entry.field("writeLatencyMillis").asDouble()); - deploymentMetricByZone.put(zone, deploymentMetrics); - }); - return deploymentMetricByZone; - } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java index 6dce576e8a4..19aa247edb4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java @@ -58,8 +58,6 @@ public class ControllerAuthorizationFilterTest { ControllerAuthorizationFilter filter = createFilter(new ControllerTester()); assertIsAllowed(invokeFilter(filter, createRequest(PUT, "/application/v4/user", USER))); assertIsAllowed(invokeFilter(filter, createRequest(POST, "/application/v4/tenant/john", USER))); - assertIsAllowed(invokeFilter(filter, createRequest(POST, "/metricforwarding/v1/deploymentmetrics", USER))); - assertIsAllowed(invokeFilter(filter, createRequest(POST, "/contactinfo/v1/tenant/john/etc", USER))); } @Test diff --git a/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp b/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp index ebd51c82794..9c9c10c1a79 100644 --- a/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp @@ -37,19 +37,26 @@ const MapDataType *verifyMapType(const DataType& type) { struct Hasher { Hasher(const MapFieldValue::IArray * keys) : _keys(keys) {} - uint32_t operator () (uint32_t index) const { return (*_keys)[index].hash(); } - const MapFieldValue::IArray * _keys; -}; - -struct Extract { - Extract(const MapFieldValue::IArray * keys) : _keys(keys) {} - const FieldValue & operator () (uint32_t index) const { return (*_keys)[index]; } + uint32_t operator () (uint32_t index) const { + return (*_keys)[index].hash(); + } + uint32_t operator () (const FieldValue & fv) const { + return fv.hash(); + } const MapFieldValue::IArray * _keys; }; struct Equal { Equal(const MapFieldValue::IArray * keys) : _keys(keys) {} - bool operator () (uint32_t a, uint32_t b) const { return (*_keys)[a].fastCompare((*_keys)[b]) == 0; } + bool operator () (uint32_t a, uint32_t b) const { + return (*_keys)[a].fastCompare((*_keys)[b]) == 0; + } + bool operator () (const FieldValue & a, uint32_t b) const { + return a.fastCompare((*_keys)[b]) == 0; + } + bool operator () (uint32_t a, const FieldValue & b) const { + return (*_keys)[a].fastCompare(b) == 0; + } const MapFieldValue::IArray * _keys; }; @@ -387,8 +394,7 @@ MapFieldValue::findIndex(const FieldValue& key) const { if ((size() > 0) && (key.getClass().id() == (*_keys)[0].getClass().id())) { ensureLookupMap(); - Extract extract(_keys.get()); - auto found = _lookupMap->find<FieldValue, Extract, vespalib::hash<FieldValue>, std::equal_to<FieldValue>>(key, extract); + auto found = _lookupMap->find(key); if (found != _lookupMap->end()) { uint32_t index = *found; assert(_present[index]); diff --git a/juniper/src/vespa/juniper/stringmap.cpp b/juniper/src/vespa/juniper/stringmap.cpp index 13d7bdee980..f673f22e29c 100644 --- a/juniper/src/vespa/juniper/stringmap.cpp +++ b/juniper/src/vespa/juniper/stringmap.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "stringmap.h" +#include <vespa/vespalib/stllike/hashtable.hpp> void Fast_StringMap::Insert(const char* key, const char* value) { @@ -9,7 +10,7 @@ void Fast_StringMap::Insert(const char* key, const char* value) const char * -Fast_StringMap::Lookup(const char *key, const char *defval) +Fast_StringMap::Lookup(const char *key, const char *defval) const { Map::const_iterator found(_backing.find(key)); return (found != _backing.end()) ? found->second.c_str() : defval; diff --git a/juniper/src/vespa/juniper/stringmap.h b/juniper/src/vespa/juniper/stringmap.h index c202ecb5888..068eeaf3d16 100644 --- a/juniper/src/vespa/juniper/stringmap.h +++ b/juniper/src/vespa/juniper/stringmap.h @@ -10,10 +10,10 @@ class Fast_StringMap { private: - typedef vespalib::hash_map<vespalib::string, vespalib::string> Map; + using Map = vespalib::hash_map<vespalib::string, vespalib::string>; Map _backing; public: void Insert(const char* key, const char* value); - const char *Lookup(const char* key, const char* defval); + const char *Lookup(const char* key, const char* defval) const; }; diff --git a/metrics/src/vespa/metrics/CMakeLists.txt b/metrics/src/vespa/metrics/CMakeLists.txt index 147e88cd61c..96156dc84b0 100644 --- a/metrics/src/vespa/metrics/CMakeLists.txt +++ b/metrics/src/vespa/metrics/CMakeLists.txt @@ -12,7 +12,6 @@ vespa_add_library(metrics metricsnapshot.cpp metrictimer.cpp metricvalueset.cpp - namehash.cpp printutils.cpp name_repo.cpp state_api_adapter.cpp diff --git a/metrics/src/vespa/metrics/loadmetric.hpp b/metrics/src/vespa/metrics/loadmetric.hpp index ce93c761a05..65098662e04 100644 --- a/metrics/src/vespa/metrics/loadmetric.hpp +++ b/metrics/src/vespa/metrics/loadmetric.hpp @@ -55,7 +55,7 @@ LoadMetric<MetricType>::LoadMetric(const LoadMetric<MetricType>& other, MetricSe } template<typename MetricType> -LoadMetric<MetricType>::~LoadMetric() { } +LoadMetric<MetricType>::~LoadMetric() = default; template<typename MetricType> MetricSet* @@ -74,10 +74,9 @@ MetricType& LoadMetric<MetricType>::getMetric(const LoadType& type) { MetricType* metric; - typename vespalib::hash_map<uint32_t, MetricTypeUP>::iterator it( - _metrics.find(type.getId())); + auto it = _metrics.find(type.getId()); if (it == _metrics.end()) { - it = _metrics.find(0); + it = _metrics.find(0u); assert(it != _metrics.end()); // Default should always exist } metric = it->second.get(); diff --git a/metrics/src/vespa/metrics/memoryconsumption.cpp b/metrics/src/vespa/metrics/memoryconsumption.cpp index 0e69defa558..0391e496ecd 100644 --- a/metrics/src/vespa/metrics/memoryconsumption.cpp +++ b/metrics/src/vespa/metrics/memoryconsumption.cpp @@ -1,11 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "memoryconsumption.h" -#include <vespa/vespalib/stllike/hash_set.h> +#include <vespa/vespalib/stllike/hash_set.hpp> #include <sstream> namespace metrics { -struct SeenStrings : public vespalib::hash_set<const void*> { }; +struct SeenStrings : public vespalib::hash_set<const char*> { }; struct SnapShotUsage : public std::vector<std::pair<std::string, uint32_t> > { }; MemoryConsumption::MemoryConsumption() @@ -16,7 +16,7 @@ MemoryConsumption::MemoryConsumption() _seenStrings->resize(1000); } -MemoryConsumption::~MemoryConsumption() { } +MemoryConsumption::~MemoryConsumption() = default; uint32_t MemoryConsumption::getStringMemoryUsage(const std::string& s, uint32_t& uniqueCount) { diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp index 5b716e2698f..7125446c168 100644 --- a/metrics/src/vespa/metrics/metricmanager.cpp +++ b/metrics/src/vespa/metrics/metricmanager.cpp @@ -13,6 +13,7 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <vespa/vespalib/stllike/hashtable.hpp> #include <sstream> #include <algorithm> @@ -23,8 +24,8 @@ namespace metrics { typedef MetricsmanagerConfig Config; -MetricManager::ConsumerSpec::ConsumerSpec() : includedMetrics() { } -MetricManager::ConsumerSpec::~ConsumerSpec() { } +MetricManager::ConsumerSpec::ConsumerSpec() = default; +MetricManager::ConsumerSpec::~ConsumerSpec() = default; void MetricManager::assertMetricLockLocked(const MetricLockGuard& g) const { diff --git a/metrics/src/vespa/metrics/namehash.cpp b/metrics/src/vespa/metrics/namehash.cpp deleted file mode 100644 index bd0b3a05697..00000000000 --- a/metrics/src/vespa/metrics/namehash.cpp +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "namehash.h" -#include "memoryconsumption.h" -#include <vespa/vespalib/stllike/hash_set.h> - -namespace metrics { - -struct NameSet : public vespalib::hash_set<std::string> { }; - -NameHash::NameHash() - : _hash(std::make_unique<NameSet>()), - _unifiedCounter(0), - _checkedCounter(0) -{ } - -NameHash::~NameHash() { } - -void -NameHash::updateName(std::string& name) { - ++_checkedCounter; - NameSet::const_iterator it(_hash->find(name)); - if (it != _hash->end()) { - if (name.c_str() != it->c_str()) { - name = *it; - ++_unifiedCounter; - } - } else { - _hash->insert(name); - } -} - -void -NameHash::addMemoryUsage(MemoryConsumption& mc) const { - mc._nameHash += sizeof(NameHash) - + _hash->getMemoryConsumption() - - sizeof(NameSet); - for (const std::string & name : *_hash) { - mc._nameHashStrings += mc.getStringMemoryUsage(name, mc._nameHashUnique); - } -} - -} // metrics diff --git a/metrics/src/vespa/metrics/namehash.h b/metrics/src/vespa/metrics/namehash.h deleted file mode 100644 index 94b4c984f6b..00000000000 --- a/metrics/src/vespa/metrics/namehash.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/** - * \class metrics::NameHash - * \ingroup metrics - * - * \brief Simple class to enable string reference counting to work better. - * - * When creating metrics, it is easy to use const char references from code, - * for instance having a for loop setting up metrics for each thread, this will - * not actually generate ref counted strings, but rather unique strings. - * - * Also, with ref counted strings, it is easy to screw it up if you access the - * string in a way requiring copy. - * - * This class is used to just keep a set of strings, and having a class for - * users to input their strings and get the "master" string with that content. - * - * Metrics use this after having registered metrics, to ensure we dont keep more - * copies of non-unique strings than needed. - */ -#pragma once - -#include "memoryconsumption.h" - -namespace metrics { - -class NameSet; - -class NameHash { - std::unique_ptr<NameSet> _hash; - uint32_t _unifiedCounter; - uint32_t _checkedCounter; - -public: - NameHash(const NameHash &) = delete; - NameHash & operator = (const NameHash &) = delete; - NameHash(); - ~NameHash(); - - void updateName(std::string& name); - - uint32_t getUnifiedStringCount() const { return _unifiedCounter; } - uint32_t getCheckedStringCount() const { return _checkedCounter; } - void resetCounts() { _unifiedCounter = 0; _checkedCounter = 0; } - void addMemoryUsage(MemoryConsumption& mc) const; -}; - -} // metrics diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index a849cef31ff..8b8294d530a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -18,6 +18,7 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoredumpHandler; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.time.Duration; import java.time.Instant; import java.time.ZoneOffset; @@ -36,6 +37,7 @@ import java.util.regex.Pattern; import static com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder.nameMatches; import static com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder.olderThan; import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.uncheck; +import static com.yahoo.vespa.hosted.node.admin.util.SecretAgentCheckConfig.nodeTypeToRole; /** * @author freva @@ -110,10 +112,16 @@ public class StorageMaintainer { if (context.nodeType() == NodeType.config || context.nodeType() == NodeType.controller) { // configserver Path configServerCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ymonsb2"); - configs.add(new SecretAgentCheckConfig(SecretAgentCheckConfig.nodeTypeToRole(context.nodeType()), 60, configServerCheckPath, + configs.add(new SecretAgentCheckConfig(nodeTypeToRole(context.nodeType()), 60, configServerCheckPath, "-zero", "configserver") .withTags(tags)); + // configserver-new + Path configServerNewCheckPath = Paths.get("curl"); + configs.add(new SecretAgentCheckConfig(nodeTypeToRole(context.nodeType())+"-new", 60, configServerNewCheckPath, + "-s", "localhost:19071/yamas-metrics") + .withTags(tags)); + //zkbackupage Path zkbackupCheckPath = context.pathInNodeUnderVespaHome("libexec/yamas2/yms_check_file_age.py"); configs.add(new SecretAgentCheckConfig("zkbackupage", 300, zkbackupCheckPath, @@ -151,7 +159,7 @@ public class StorageMaintainer { private Map<String, Object> generateTags(NodeAgentContext context, NodeSpec node) { Map<String, String> tags = new LinkedHashMap<>(); tags.put("namespace", "Vespa"); - tags.put("role", SecretAgentCheckConfig.nodeTypeToRole(node.getNodeType())); + tags.put("role", nodeTypeToRole(node.getNodeType())); tags.put("zone", String.format("%s.%s", context.zoneId().environment().value(), context.zoneId().regionName().value())); node.getVespaVersion().ifPresent(version -> tags.put("vespaVersion", version)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index d276bad9521..cf5d29d70f1 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -117,8 +117,8 @@ public class StorageMaintainerTest { public void configserver() { Path path = executeAs(NodeType.config); - assertChecks(path, "athenz-certificate-expiry", "configserver", "host-life", "ntp", - "system-coredumps-processing", "zkbackupage"); + assertChecks(path, "athenz-certificate-expiry", "configserver", "configserver-new", + "host-life", "ntp", "system-coredumps-processing", "zkbackupage"); assertCheckEnds(path.resolve("configserver.yaml"), " tags:\n" + @@ -132,8 +132,8 @@ public class StorageMaintainerTest { public void controller() { Path path = executeAs(NodeType.controller); - assertChecks(path, "athenz-certificate-expiry", "controller", "host-life", "ntp", - "system-coredumps-processing", "vespa", "vespa-health", "zkbackupage"); + assertChecks(path, "athenz-certificate-expiry", "controller", "controller-new", "host-life", + "ntp", "system-coredumps-processing", "vespa", "vespa-health", "zkbackupage"); // Do not set namespace for vespa metrics. WHY? diff --git a/searchcommon/src/vespa/searchcommon/common/schema.cpp b/searchcommon/src/vespa/searchcommon/common/schema.cpp index 5e9db03bf31..cef74409024 100644 --- a/searchcommon/src/vespa/searchcommon/common/schema.cpp +++ b/searchcommon/src/vespa/searchcommon/common/schema.cpp @@ -4,6 +4,7 @@ #include <fstream> #include <vespa/config/common/configparser.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <vespa/vespalib/stllike/hashtable.hpp> #include <vespa/fastos/file.h> #include <vespa/log/log.h> @@ -421,6 +422,25 @@ Schema::getFieldSetId(vespalib::stringref name) const return getFieldId(name, _fieldSetIds); } +bool +Schema::isIndexField(vespalib::stringref name) const +{ + return _indexIds.find(name) != _indexIds.end(); +} + +bool +Schema::isSummaryField(vespalib::stringref name) const +{ + return _summaryIds.find(name) != _summaryIds.end(); +} + +bool +Schema::isAttributeField(vespalib::stringref name) const +{ + return _attributeIds.find(name) != _attributeIds.end(); +} + + void Schema::swap(Schema &rhs) { diff --git a/searchcommon/src/vespa/searchcommon/common/schema.h b/searchcommon/src/vespa/searchcommon/common/schema.h index d066ccb0a27..f8020d6ed9a 100644 --- a/searchcommon/src/vespa/searchcommon/common/schema.h +++ b/searchcommon/src/vespa/searchcommon/common/schema.h @@ -293,11 +293,7 @@ public: * @return true if field is an index field. * @param name the name of the field. **/ - bool - isIndexField(vespalib::stringref name) const - { - return _indexIds.find(name) != _indexIds.end(); - } + bool isIndexField(vespalib::stringref name) const; /** * Check if a field is a summary field @@ -305,22 +301,15 @@ public: * @return true if field is an summary field. * @param name the name of the field. **/ - bool - isSummaryField(vespalib::stringref name) const - { - return _summaryIds.find(name) != _summaryIds.end(); - } + bool isSummaryField(vespalib::stringref name) const; + /** * Check if a field is a attribute field * * @return true if field is an attribute field. * @param name the name of the field. **/ - bool - isAttributeField(vespalib::stringref name) const - { - return _attributeIds.find(name) != _attributeIds.end(); - } + bool isAttributeField(vespalib::stringref name) const; /** * Get information about a specific attribute field using the given fieldId. diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp index 577d6e7edf5..16308ed22f9 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp @@ -6,6 +6,7 @@ #include "fnet_search.h" #include "mergehits.h" #include <vespa/searchlib/engine/packetconverter.h> +#include <vespa/searchlib/engine/searchreply.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/xxhash/xxhash.h> @@ -870,6 +871,7 @@ FastS_FNET_Search::CheckCoverage() uint16_t nodesQueried = 0; uint16_t nodesReplied = 0; size_t cntNone(0); + size_t askedButNotAnswered(0); for (const FastS_FNET_SearchNode & node : _nodes) { if (node._qresult != nullptr) { @@ -882,14 +884,21 @@ FastS_FNET_Search::CheckCoverage() } else { nodesQueried++; cntNone++; + if (node.IsConnected()) { + askedButNotAnswered++; + } } } + bool missingReplies = (askedButNotAnswered != 0) || (nodesQueried != nodesReplied); const ssize_t missingParts = cntNone - (_dataset->getSearchableCopies() - 1); - if ((missingParts > 0) && (cntNone != _nodes.size())) { + if (((missingParts > 0) && (cntNone != _nodes.size())) || (missingReplies && useAdaptiveTimeout())) { // TODO This is a dirty way of anticipating missing coverage. // It should be done differently activeDocs += missingParts * activeDocs/(_nodes.size() - cntNone); } + if (missingReplies && useAdaptiveTimeout()) { + degradedReason |= search::engine::SearchReply::Coverage::ADAPTIVE_TIMEOUT; + } _util.SetCoverage(covDocs, activeDocs, soonActiveDocs, degradedReason, nodesQueried, nodesReplied); } @@ -1446,22 +1455,26 @@ FastS_FNET_Search::ProcessDocsumsDone() return RET_OK; } +bool +FastS_FNET_Search::useAdaptiveTimeout() const { + return _dataset->getMinimalSearchCoverage() < 100.0; +} void FastS_FNET_Search::adjustQueryTimeout() { uint32_t pendingQueries = getPendingQueries(); - if (pendingQueries == 0 || _util.IsQueryFlagSet(search::fs4transport::QFLAG_DUMP_FEATURES)) { + if ((pendingQueries == 0) || + _util.IsQueryFlagSet(search::fs4transport::QFLAG_DUMP_FEATURES) || + ! useAdaptiveTimeout()) + { return; } double mincoverage = _dataset->getMinimalSearchCoverage(); - uint32_t wantedAnswers = getRequestedQueries(); - if (mincoverage < 100.0) { - wantedAnswers *= mincoverage / 100.0; - LOG(spam, "Adjusting wanted answers from %u to %u", getRequestedQueries(), wantedAnswers); - } + uint32_t wantedAnswers = getRequestedQueries() * mincoverage / 100.0; + LOG(spam, "Adjusting wanted answers from %u to %u", getRequestedQueries(), wantedAnswers); if (getDoneQueries() < wantedAnswers) { return; } diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h index 0ad9a7f0067..81e84e92bbd 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h +++ b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h @@ -328,6 +328,7 @@ public: char *ST_GetSortData() { return _util.GetSortData(); } FastS_QueryResult *ST_GetQueryResult() { return _util.GetQueryResult(); } + bool useAdaptiveTimeout() const; void adjustQueryTimeout(); void adjustDocsumTimeout(); uint32_t getRequestedQueries() const { return _queryNodes; } diff --git a/searchlib/src/vespa/searchlib/aggregation/group.cpp b/searchlib/src/vespa/searchlib/aggregation/group.cpp index 6eb131fe77d..17e56c9af21 100644 --- a/searchlib/src/vespa/searchlib/aggregation/group.cpp +++ b/searchlib/src/vespa/searchlib/aggregation/group.cpp @@ -34,8 +34,8 @@ struct SortByGroupRank { } }; -void reset(Group * & v) { v = NULL; } -void destruct(Group * v) { if (v) { delete v; } } +void reset(Group * & v) { v = nullptr; } +void destruct(Group * v) { delete v; } void destruct(Group::GroupList & l, size_t m) @@ -44,7 +44,7 @@ destruct(Group::GroupList & l, size_t m) destruct(l[i]); } delete [] l; - l = NULL; + l = nullptr; } } @@ -97,13 +97,13 @@ Group::groupNext(const GroupingLevel & level, const Doc & doc, HitRank rank) Group * Group::Value::groupSingle(const ResultNode & selectResult, HitRank rank, const GroupingLevel & level) { - if (_childInfo._childMap == NULL) { + if (_childInfo._childMap == nullptr) { assert(getChildrenSize() == 0); _childInfo._childMap = new GroupHash(1, GroupHasher(&_children), GroupEqual(&_children)); } GroupHash & childMap = *_childInfo._childMap; - Group * group(NULL); - GroupHash::iterator found = childMap.find<ResultNode, GroupResult, ResultHash, ResultEqual>(selectResult, GroupResult(&_children)); + Group * group(nullptr); + GroupHash::iterator found = childMap.find(selectResult); if (found == childMap.end()) { // group not present in child map if (level.allowMoreGroups(childMap.size())) { group = new Group(level.getGroupPrototype()); @@ -203,7 +203,7 @@ Group::Group() : Group::Group(const Group & rhs) = default; Group & Group::operator = (const Group & rhs) = default; -Group::~Group() { } +Group::~Group() = default; Group & Group::partialCopy(const Group & rhs) { @@ -230,7 +230,7 @@ void Group::Value::addExpressionResult(ExpressionNode::UP expressionNode) { uint32_t newSize = getAggrSize() + getExprSize() + 1; - ExpressionVector n = new ExpressionNode::CP[newSize]; + auto n = new ExpressionNode::CP[newSize]; for (uint32_t i(0); i < (newSize - 1); i++) { n[i] = std::move(_aggregationResults[i]); } @@ -246,7 +246,7 @@ Group::Value::addAggregationResult(ExpressionNode::UP aggr) { assert(getAggrSize() < 15); size_t newSize = getAggrSize() + 1 + getExprSize(); - ExpressionVector n = new ExpressionNode::CP[newSize]; + auto n = new ExpressionNode::CP[newSize]; for (size_t i(0), m(getAggrSize()); i < m; i++) { n[i] = std::move(_aggregationResults[i]); } @@ -292,10 +292,10 @@ Group::Value::addChild(Group * child) { const size_t sz(getChildrenSize()); assert(sz < 0xffffff); - if (_children == 0) { + if (_children == nullptr) { _children = new ChildP[4]; } else if ((sz >=4) && vespalib::Optimized::msbIdx(sz) == vespalib::Optimized::lsbIdx(sz)) { - GroupList n = new ChildP[sz*2]; + auto n = new ChildP[sz*2]; for (size_t i(0), m(getChildrenSize()); i < m; i++) { n[i] = _children[i]; } @@ -317,7 +317,7 @@ Group::Value::select(const vespalib::ObjectPredicate &predicate, vespalib::Objec void Group::Value::preAggregate() { - assert(_childInfo._childMap == NULL); + assert(_childInfo._childMap == nullptr); _childInfo._childMap = new GroupHash(getChildrenSize()*2, GroupHasher(&_children), GroupEqual(&_children)); GroupHash & childMap = *_childInfo._childMap; for (ChildP *it(_children), *mt(_children + getChildrenSize()); it != mt; ++it) { @@ -330,7 +330,7 @@ void Group::Value::postAggregate() { delete _childInfo._childMap; - _childInfo._childMap = NULL; + _childInfo._childMap = nullptr; for (ChildP *it(_children), *mt(_children + getChildrenSize()); it != mt; ++it) { (*it)->postAggregate(); } @@ -372,7 +372,7 @@ Group::Value::execute() { void Group::Value::mergeLevel(const Group & protoType, const Value & b) { for (ChildP *it(b._children), *mt(b._children + b.getChildrenSize()); it != mt; ++it) { - ChildP g(new Group(protoType)); + auto g(new Group(protoType)); g->partialCopy(**it); addChild(g); } @@ -382,7 +382,7 @@ void Group::Value::merge(const std::vector<GroupingLevel> &levels, uint32_t firstLevel, uint32_t currentLevel, const Value &b) { - GroupList z = new ChildP[getChildrenSize() + b.getChildrenSize()]; + auto z = new ChildP[getChildrenSize() + b.getChildrenSize()]; size_t kept(0); ChildP * px = _children; ChildP * ex = _children + getChildrenSize(); @@ -422,7 +422,7 @@ Group::Value::merge(const std::vector<GroupingLevel> &levels, void Group::Value::prune(const Value & b, uint32_t lastLevel, uint32_t currentLevel) { - GroupList keep = new ChildP[b.getChildrenSize()]; + auto keep = new ChildP[b.getChildrenSize()]; size_t kept(0); ChildP * px = _children; ChildP * ex = _children + getAllChildrenSize(); @@ -562,7 +562,7 @@ Group::Value::deserialize(Deserializer & is) { // results into a temporary buffer, and then reallocate the actual // vector when we know the total size. Then we copy the temp buffer and // deserialize the rest to the end of the vector. - ExpressionVector tmpAggregationResults = new ExpressionNode::CP[aggrSize]; + auto tmpAggregationResults = new ExpressionNode::CP[aggrSize]; setAggrSize(aggrSize); for(uint32_t i(0); i < aggrSize; i++) { is >> tmpAggregationResults[i]; @@ -589,7 +589,7 @@ Group::Value::deserialize(Deserializer & is) { _children = new ChildP[std::max(4ul, 2ul << vespalib::Optimized::msbIdx(count))]; setChildrenSize(count); for(uint32_t i(0); i < count; i++) { - ChildP group(new Group); + auto group(new Group); is >> *group; _children[i] = group; } @@ -633,24 +633,24 @@ Group::Value::visitMembers(vespalib::ObjectVisitor &visitor) const { Group::Value::Value() : _packedLength(0), _tag(-1), - _aggregationResults(NULL), - _children(NULL), + _aggregationResults(nullptr), + _children(nullptr), _childInfo(), _orderBy() { memset(_orderBy, 0, sizeof(_orderBy)); - _childInfo._childMap = NULL; + _childInfo._childMap = nullptr; } Group::Value::Value(const Value & rhs) : _packedLength(rhs._packedLength), _tag(rhs._tag), - _aggregationResults(NULL), - _children(NULL), + _aggregationResults(nullptr), + _children(nullptr), _childInfo(), _orderBy() { - _childInfo._childMap = NULL; + _childInfo._childMap = nullptr; memcpy(_orderBy, rhs._orderBy, sizeof(_orderBy)); uint32_t totalAggrSize = rhs.getAggrSize() + rhs.getExprSize(); if (totalAggrSize > 0) { @@ -665,17 +665,17 @@ Group::Value::Value(const Value & rhs) : _children = new ChildP[std::max(4ul, 2ul << vespalib::Optimized::msbIdx(rhs.getChildrenSize()))]; size_t i(0); for (const ChildP *it(rhs._children), *mt(rhs._children + rhs.getChildrenSize()); it != mt; ++it, i++) { - _children[i] = ChildP(new Group(**it)); + _children[i] = new Group(**it); } } } Group::Value::Value(Value && rhs) noexcept : - _packedLength(std::move(rhs._packedLength)), - _tag(std::move(rhs._tag)), - _aggregationResults(std::move(rhs._aggregationResults)), - _children(std::move(rhs._children)), - _childInfo(std::move(rhs._childInfo)), + _packedLength(rhs._packedLength), + _tag(rhs._tag), + _aggregationResults(rhs._aggregationResults), + _children(rhs._children), + _childInfo(rhs._childInfo), _orderBy() { memcpy(_orderBy, rhs._orderBy, sizeof(_orderBy)); @@ -688,11 +688,11 @@ Group::Value::Value(Value && rhs) noexcept : Group::Value & Group::Value::operator =(Value && rhs) noexcept { - _packedLength = std::move(rhs._packedLength); - _tag = std::move(rhs._tag); - _aggregationResults = std::move(rhs._aggregationResults); - _children = std::move(rhs._children); - _childInfo = std::move(rhs._childInfo); + _packedLength = rhs._packedLength; + _tag = rhs._tag; + _aggregationResults = rhs._aggregationResults; + _children = rhs._children; + _childInfo = rhs._childInfo; memcpy(_orderBy, rhs._orderBy, sizeof(_orderBy)); rhs.setChildrenSize(0); diff --git a/searchlib/src/vespa/searchlib/aggregation/group.h b/searchlib/src/vespa/searchlib/aggregation/group.h index c769b6c1d27..f302346f211 100644 --- a/searchlib/src/vespa/searchlib/aggregation/group.h +++ b/searchlib/src/vespa/searchlib/aggregation/group.h @@ -42,26 +42,18 @@ public: struct GroupEqual : public std::binary_function<ChildP, ChildP, bool> { GroupEqual(const GroupList * v) : _v(v) { } bool operator()(uint32_t a, uint32_t b) { return (*_v)[a]->getId().cmpFast((*_v)[b]->getId()) == 0; } + bool operator()(const Group & a, uint32_t b) { return a.getId().cmpFast((*_v)[b]->getId()) == 0; } + bool operator()(uint32_t a, const Group & b) { return (*_v)[a]->getId().cmpFast(b.getId()) == 0; } + bool operator()(const ResultNode & a, uint32_t b) { return a.cmpFast((*_v)[b]->getId()) == 0; } + bool operator()(uint32_t a, const ResultNode & b) { return (*_v)[a]->getId().cmpFast(b) == 0; } const GroupList *_v; }; struct GroupHasher { GroupHasher(const GroupList * v) : _v(v) { } size_t operator() (uint32_t arg) const { return (*_v)[arg]->getId().hash(); } - const GroupList *_v; - }; - struct GroupResult { - GroupResult(const GroupList * v) : _v(v) { } - const ResultNode & operator() (uint32_t arg) const { return (*_v)[arg]->getId(); } - const GroupList *_v; - }; - struct ResultLess : public std::binary_function<ResultNode::CP, ResultNode::CP, bool> { - bool operator()(const ResultNode::CP & a, const ResultNode::CP & b) { return a->cmpFast(*b) < 0; } - }; - struct ResultEqual : public std::binary_function<ResultNode, ResultNode, bool> { - bool operator()(const ResultNode & a, const ResultNode & b) { return a.cmpFast(b) == 0; } - }; - struct ResultHash { + size_t operator() (const Group & arg) const { return arg.getId().hash(); } size_t operator() (const ResultNode & arg) const { return arg.hash(); } + const GroupList *_v; }; using GroupingLevelList = std::vector<GroupingLevel>; @@ -192,12 +184,12 @@ public: return _aggr.groupSingle(result, rank, level); } - bool hasId() const { return (_id.get() != NULL); } + bool hasId() const { return static_cast<bool>(_id); } const ResultNode &getId() const { return *_id; } Group unchain() const { return *this; } - Group &setId(const ResultNode &id) { _id.reset(static_cast<ResultNode *>(id.clone())); return *this; } + Group &setId(const ResultNode &id) { _id.reset(id.clone()); return *this; } Group &addAggregationResult(ExpressionNode::UP result) { _aggr.addAggregationResult(std::move(result)); return *this; diff --git a/searchlib/src/vespa/searchlib/engine/searchreply.h b/searchlib/src/vespa/searchlib/engine/searchreply.h index 9135866a7f7..40dca8a1c46 100644 --- a/searchlib/src/vespa/searchlib/engine/searchreply.h +++ b/searchlib/src/vespa/searchlib/engine/searchreply.h @@ -53,8 +53,8 @@ public: Coverage & degradeMatchPhase() { _degradeReason |= MATCH_PHASE; return *this; } Coverage & degradeTimeout() { _degradeReason |= TIMEOUT; return *this; } Coverage & degradeAdaptiveTimeout() { _degradeReason |= ADAPTIVE_TIMEOUT; return *this; } - private: enum DegradeReason {MATCH_PHASE=0x01, TIMEOUT=0x02, ADAPTIVE_TIMEOUT=0x04}; + private: uint64_t _covered; uint64_t _active; uint64_t _soonActive; diff --git a/searchlib/src/vespa/searchlib/grouping/groupengine.cpp b/searchlib/src/vespa/searchlib/grouping/groupengine.cpp index aa289da068b..b652d5a840d 100644 --- a/searchlib/src/vespa/searchlib/grouping/groupengine.cpp +++ b/searchlib/src/vespa/searchlib/grouping/groupengine.cpp @@ -6,12 +6,10 @@ #include <vespa/vespalib/stllike/hash_set.hpp> #include <cassert> -namespace search { +using namespace search::expression; +using namespace search::aggregation; -using namespace expression; -using namespace aggregation; - -namespace grouping { +namespace search::grouping { GroupEngine::GroupEngine(const GroupingLevel * request, size_t level, GroupEngine * nextEngine, bool frozen) : Collect(request->getGroupPrototype()), @@ -52,7 +50,7 @@ GroupRef GroupEngine::group(Children & children, uint32_t docId, double rank) throw std::runtime_error("Does not know how to handle failed select statements"); } const ResultNode &selectResult = selector.getResult(); - Children::iterator found = children.find<ResultNode, GroupResult, Group::ResultHash, Group::ResultEqual>(selectResult, GroupResult(*this)); + Children::iterator found = children.find(selectResult); GroupRef gr; if (found == children.end()) { if (_request->allowMoreGroups(children.size())) { @@ -228,7 +226,6 @@ GroupEngine::preFillEngine(const Group & r, size_t depth) } } -} // this function was added by ../../forcelink.sh void forcelink_file_searchlib_grouping_groupengine() {} diff --git a/searchlib/src/vespa/searchlib/grouping/groupengine.h b/searchlib/src/vespa/searchlib/grouping/groupengine.h index a4f6d7a43f6..5167c37560a 100644 --- a/searchlib/src/vespa/searchlib/grouping/groupengine.h +++ b/searchlib/src/vespa/searchlib/grouping/groupengine.h @@ -5,8 +5,7 @@ #include <vespa/searchlib/grouping/collect.h> #include <vespa/vespalib/util/sort.h> -namespace search { -namespace grouping { +namespace search::grouping { class GroupEngine : protected Collect { @@ -15,6 +14,7 @@ public: public: GroupHash(const GroupEngine & engine) : _engine(engine) { } uint32_t operator () (GroupRef a) const { return _engine.hash(a); } + uint32_t operator () (const expression::ResultNode & a) const { return a.hash(); } private: const GroupEngine & _engine; }; @@ -22,6 +22,8 @@ public: public: GroupEqual(const GroupEngine & engine) : _engine(engine) { } bool operator () (GroupRef a, GroupRef b) const { return _engine.cmpId(a, b) == 0; } + bool operator () (const expression::ResultNode & a, GroupRef b) const { return a.cmpFast(_engine.getGroupId(b)) == 0; } + bool operator () (GroupRef a, const expression::ResultNode & b) const { return _engine.getGroupId(a).cmpFast(b) == 0; } private: const GroupEngine & _engine; }; @@ -137,4 +139,3 @@ private: }; } -} diff --git a/searchlib/src/vespa/searchlib/util/stringenum.cpp b/searchlib/src/vespa/searchlib/util/stringenum.cpp index 60238c32cc6..efcf33e73ab 100644 --- a/searchlib/src/vespa/searchlib/util/stringenum.cpp +++ b/searchlib/src/vespa/searchlib/util/stringenum.cpp @@ -2,13 +2,13 @@ #include "stringenum.h" #include <vespa/fastlib/io/bufferedfile.h> +#include <vespa/vespalib/stllike/hashtable.hpp> #include <cassert> #include <vespa/log/log.h> LOG_SETUP(".seachlib.util.stringenum"); -namespace search { -namespace util { +namespace search::util { static inline char * StripString(char *str) @@ -32,7 +32,14 @@ StripString(char *str) return first; } -StringEnum::~StringEnum() { } +StringEnum::StringEnum() + : _numEntries(0), + _mapping(), + _reverseMap() +{ +} + +StringEnum::~StringEnum() = default; void StringEnum::CreateReverseMapping() const @@ -123,5 +130,44 @@ StringEnum::Load(const char *filename) return true; } +void +StringEnum::Clear() +{ + _reverseMap.clear(); + _mapping.clear(); + _numEntries = 0; +} + +int +StringEnum::Add(const char *str) +{ + Map::const_iterator found(_mapping.find(str)); + if (found != _mapping.end()) { + return found->second; + } else { + int value = _numEntries++; + _mapping[str] = value; + return value; + } } + +int +StringEnum::Lookup(const char *str) const +{ + Map::const_iterator found(_mapping.find(str)); + return (found != _mapping.end()) ? found->second : -1; +} + +const char * +StringEnum::Lookup(uint32_t value) const +{ + if (value >= _numEntries) + return NULL; + + if (_numEntries > _reverseMap.size()) + CreateReverseMapping(); + + return _reverseMap[value]; +} + } diff --git a/searchlib/src/vespa/searchlib/util/stringenum.h b/searchlib/src/vespa/searchlib/util/stringenum.h index 44b3afca539..bd234ba2e36 100644 --- a/searchlib/src/vespa/searchlib/util/stringenum.h +++ b/searchlib/src/vespa/searchlib/util/stringenum.h @@ -5,8 +5,7 @@ #include <vector> #include <vespa/vespalib/stllike/hash_map.h> -namespace search { -namespace util { +namespace search::util { /** * An object of this class represents an enumeration of a set of @@ -35,29 +34,17 @@ public: /** * Create an empty string enumeration. **/ - StringEnum() - : _numEntries(0), - _mapping(), - _reverseMap() - { - } + StringEnum(); /** * Destructor. **/ ~StringEnum(); - /** * Discard all entries held by this object. **/ - void Clear() - { - _reverseMap.clear(); - _mapping.clear(); - _numEntries = 0; - } - + void Clear(); /** * Add a string to this enumeration. Equal strings will get the same @@ -68,18 +55,7 @@ public: * @return the enumerated value for the given string. * @param str string you want to add. **/ - int Add(const char *str) - { - Map::const_iterator found(_mapping.find(str)); - if (found != _mapping.end()) { - return found->second; - } else { - int value = _numEntries++; - _mapping[str] = value; - return value; - } - } - + int Add(const char *str); /** * Obtain the enumerated value for the given string. @@ -87,12 +63,7 @@ public: * @return enumerated value or -1 if not present. * @param str the string to look up. **/ - int Lookup(const char *str) const - { - Map::const_iterator found(_mapping.find(str)); - return (found != _mapping.end()) ? found->second : -1; - } - + int Lookup(const char *str) const; /** * Obtain the string for the given enumerated value. @@ -100,17 +71,7 @@ public: * @return string or NULL if out of range. * @param value the enumerated value to look up. **/ - const char *Lookup(uint32_t value) const - { - if (value >= _numEntries) - return NULL; - - if (_numEntries > _reverseMap.size()) - CreateReverseMapping(); - - return _reverseMap[value]; - } - + const char *Lookup(uint32_t value) const; /** * Obtain the number of entries currently present in this @@ -141,5 +102,3 @@ public: }; } -} - diff --git a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp index d75ca47dd33..79266d34585 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp @@ -4,6 +4,7 @@ #include "keywordextractor.h" #include "idocsumenvironment.h" #include <vespa/searchlib/parsequery/stackdumpiterator.h> +#include <vespa/vespalib/stllike/hashtable.hpp> /** Tell us what parts of the query we are interested in */ @@ -12,12 +13,7 @@ namespace search::docsummary { bool useful(search::ParseItem::ItemCreator creator) { - switch (creator) { - case search::ParseItem::CREA_ORIG: - return true; - default: - return false; - } + return creator == search::ParseItem::CREA_ORIG; } @@ -38,6 +34,12 @@ KeywordExtractor::~KeywordExtractor() } } +bool +KeywordExtractor::IsLegalIndexName(const char *idxName) const +{ + return _legalIndexes.find(idxName) != _legalIndexes.end(); +} + KeywordExtractor::IndexPrefix::IndexPrefix(const char *prefix, IndexPrefix **list) : _prefix(NULL), _prefixLen(0), diff --git a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h index 6c9ed675225..cfc73d606a0 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h @@ -51,11 +51,7 @@ private: return false; } - bool IsLegalIndexName(const char *idxName) const - { - return _legalIndexes.find(idxName) != _legalIndexes.end(); - } - + bool IsLegalIndexName(const char *idxName) const; public: explicit KeywordExtractor(IDocsumEnvironment * env); ~KeywordExtractor(); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp index 507ec7b3866..8066a5e65db 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp @@ -2,6 +2,7 @@ #include "resultclass.h" #include "resultconfig.h" +#include <vespa/vespalib/stllike/hashtable.hpp> #include <cassert> #include <zlib.h> @@ -18,8 +19,14 @@ ResultClass::ResultClass(const char *name, uint32_t id, util::StringEnum & field { } -ResultClass::~ResultClass() { } +ResultClass::~ResultClass() = default; +int +ResultClass::GetIndexFromName(const char* name) const +{ + NameIdMap::const_iterator found(_nameMap.find(name)); + return (found != _nameMap.end()) ? found->second : -1; +} bool ResultClass::AddConfigEntry(const char *name, ResType type) diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h index d1504ed5bdd..e7c7c799b5f 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.h @@ -245,11 +245,7 @@ public: * * @return field index or -1 if not found. **/ - int GetIndexFromName(const char* name) const - { - NameIdMap::const_iterator found(_nameMap.find(name)); - return (found != _nameMap.end()) ? found->second : -1; - } + int GetIndexFromName(const char* name) const; /** diff --git a/staging_vespalib/src/vespa/vespalib/objects/identifiable.cpp b/staging_vespalib/src/vespa/vespalib/objects/identifiable.cpp index 7e14fbc0014..84bdc0a45d0 100644 --- a/staging_vespalib/src/vespa/vespalib/objects/identifiable.cpp +++ b/staging_vespalib/src/vespa/vespalib/objects/identifiable.cpp @@ -27,18 +27,28 @@ public: bool erase(RuntimeClass * c); const RuntimeClass * classFromId(unsigned id) const; const RuntimeClass * classFromName(const char * name) const; - const char * id2Name(unsigned id) const; - unsigned name2Id(const char * name) const; bool empty() const { return _listById.empty(); } private: - struct GetId { uint32_t operator() (const RuntimeClass * f) const { return f->id(); } }; - struct HashId { size_t operator() (const RuntimeClass * f) const { return f->id(); } }; - struct EqualId { bool operator() (const RuntimeClass * a, const RuntimeClass * b) const { return a->id() == b->id(); } }; - struct GetName { const char * operator() (const RuntimeClass * f) const { return f->name(); } }; - struct HashName { size_t operator() (const RuntimeClass * f) const { return hashValue(f->name()); } }; - struct EqualName { bool operator() (const RuntimeClass * a, const RuntimeClass * b) const { return strcmp(a->name(), b->name()) == 0; } }; - typedef hash_set<RuntimeClass *, HashId, EqualId> IdList; - typedef hash_set<RuntimeClass *, HashName, EqualName> NameList; + struct HashId { + uint32_t operator() (const RuntimeClass * f) const { return f->id(); } + uint32_t operator() (uint32_t id) const { return id; } + }; + struct EqualId { + bool operator() (const RuntimeClass * a, const RuntimeClass * b) const { return a->id() == b->id(); } + bool operator() (const RuntimeClass * a, uint32_t b) const { return a->id() == b; } + bool operator() (uint32_t a, const RuntimeClass * b) const { return a == b->id(); } + }; + struct HashName { + uint32_t operator() (const RuntimeClass * f) const { return hashValue(f->name()); } + uint32_t operator() (const char * name) const { return hashValue(name); } + }; + struct EqualName { + bool operator() (const RuntimeClass * a, const RuntimeClass * b) const { return strcmp(a->name(), b->name()) == 0; } + bool operator() (const RuntimeClass * a, const char * b) const { return strcmp(a->name(), b) == 0; } + bool operator() (const char * a, const RuntimeClass * b) const { return strcmp(a, b->name()) == 0; } + }; + using IdList = hash_set<RuntimeClass *, HashId, EqualId>; + using NameList = hash_set<RuntimeClass *, HashName, EqualName>; IdList _listById; NameList _listByName; }; @@ -69,14 +79,14 @@ bool Register::append(Identifiable::RuntimeClass * c) const Identifiable::RuntimeClass * Register::classFromId(unsigned id) const { - IdList::const_iterator it(_listById.find<uint32_t, GetId, hash<uint32_t>, std::equal_to<uint32_t> >(id)); - return (it != _listById.end()) ? *it : NULL; + IdList::const_iterator it(_listById.find<uint32_t>(id)); + return (it != _listById.end()) ? *it : nullptr; } const Identifiable::RuntimeClass * Register::classFromName(const char *name) const { - NameList::const_iterator it(_listByName.find<const char *, GetName, hash<const char *>, std::equal_to<const char *> >(name)); - return (it != _listByName.end()) ? *it : NULL; + NameList::const_iterator it(_listByName.find<const char *>(name)); + return (it != _listByName.end()) ? *it : nullptr; } Register * _register = nullptr; @@ -115,7 +125,7 @@ Identifiable::RuntimeClass::RuntimeClass(RuntimeInfo * info_) : } } } - if (_register == NULL) { + if (_register == nullptr) { _register = new Register(); } if (! _register->append(this)) { @@ -131,7 +141,7 @@ Identifiable::RuntimeClass::~RuntimeClass() } if (_register->empty()) { delete _register; - _register = NULL; + _register = nullptr; } } @@ -142,20 +152,6 @@ bool Identifiable::RuntimeClass::inherits(unsigned cid) const return (cid == cur->_id); } -class SortById : public std::binary_function<const Identifiable::RuntimeClass *, const Identifiable::RuntimeClass *, bool> { -public: - bool operator() (const Identifiable::RuntimeClass * x, const Identifiable::RuntimeClass * y) const { - return x->id() < y->id(); - } -}; - -class SortByName : public std::binary_function<const Identifiable::RuntimeClass *, const Identifiable::RuntimeClass *, bool> { -public: - bool operator() (const Identifiable::RuntimeClass * x, const Identifiable::RuntimeClass * y) const { - return strcmp(x->name(), y->name()) < 0; - } -}; - Serializer & operator << (Serializer & os, const Identifiable & obj) { os.put(Identifiable::classIdField, obj.getClass().id()); @@ -199,16 +195,16 @@ Identifiable::UP Identifiable::create(Deserializer & is) is.get(classIdField, cid); UP obj; const Identifiable::RuntimeClass *rtc = Identifiable::classFromId(cid); - if (rtc == NULL) { - if ((_classLoader != NULL) && _classLoader->hasClass(cid)) { + if (rtc == nullptr) { + if ((_classLoader != nullptr) && _classLoader->hasClass(cid)) { _classLoader->loadClass(cid); rtc = Identifiable::classFromId(cid); - if (rtc == NULL) { + if (rtc == nullptr) { throw std::runtime_error(make_string("Failed loading class for Identifiable with classId %d(%0x)", cid, cid)); } } } - if (rtc != NULL) { + if (rtc != nullptr) { obj.reset(rtc->create()); if (obj.get()) { obj->deserialize(is); diff --git a/tenant-base/pom.xml b/tenant-base/pom.xml index 51d830f0e7d..a29302f9ab4 100644 --- a/tenant-base/pom.xml +++ b/tenant-base/pom.xml @@ -78,12 +78,13 @@ <profiles> <profile> - <!-- Build *-fat-test.jar file that includes all classes and resources - that are part of the class path during test and put it inside a zip: + <!-- Build *-fat-test.jar file that includes all non-test classes and resources + that are part of the class path during test and and test.jar that includes + all test classes and resources, and put it inside a zip: 1. application classes and resources 2. test classes and resources 3. classes and resources in all dependencies of both (1) and (2) - 4. copy the fat-test-jar to application-test/artifacts directory + 4. copy the fat-test-jar and test-jar to application-test/artifacts directory 5. zip application-test --> <id>fat-test-application</id> <build> @@ -154,10 +155,6 @@ <resource> <directory>target/classes</directory> </resource> - <!-- test classes and resources, see 2. above --> - <resource> - <directory>target/test-classes</directory> - </resource> </resources> </configuration> </execution> @@ -169,6 +166,7 @@ <version>3.1.0</version> <executions> <execution> + <id>fat-test-jar</id> <phase>package</phase> <goals> <goal>jar</goal> @@ -178,6 +176,13 @@ <classifier>fat-test</classifier> </configuration> </execution> + <execution> + <id>test-jar</id> + <phase>package</phase> + <goals> + <goal>test-jar</goal> + </goals> + </execution> </executions> </plugin> <plugin> @@ -196,6 +201,10 @@ <copy file="target/${project.artifactId}-fat-test.jar" todir="target/application-test/artifacts/" /> + <!-- copy slim test-jar to application-test artifacts directory, see 4. above --> + <copy file="target/${project.artifactId}-tests.jar" + todir="target/application-test/artifacts/" /> + <!-- zip application-test, see 5. above --> <zip destfile="target/application-test.zip" basedir="target/application-test/" /> diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java index 84d3b320772..837df3c98ef 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/CommandLineArguments.java @@ -66,6 +66,16 @@ public class CommandLineArguments { return null; } } + if (cmdArgs.enableV2Protocol) { + if (cmdArgs.enableV3Protocol) { + System.err.println("both --useV2Protocol and --useV3Protocol options specified, ignoring deprecated --useV2Protocol option"); + cmdArgs.enableV2Protocol = false; + } else { + System.err.println("--useV2Protocol option is deprecated"); + } + } else { + cmdArgs.enableV3Protocol = true; + } return cmdArgs; } @@ -99,10 +109,10 @@ public class CommandLineArguments { @Inject private HelpOption helpOption; - @Option(name = {"--useV3Protocol"}, description = "Not used anymore, see useV2Protocol.") - private boolean notUsedBoolean = true; + @Option(name = {"--useV3Protocol"}, description = "Use V3 protocol to gateway. This is the default protocol.") + private boolean enableV3Protocol = false; - @Option(name = {"--useV2Protocol"}, description = "Use old V2 protocol to gateway.") + @Option(name = {"--useV2Protocol"}, description = "Use old V2 protocol to gateway. This option is deprecated.") private boolean enableV2Protocol = false; @Option(name = {"--file"}, diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/runner/CommandLineArgumentsTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/runner/CommandLineArgumentsTest.java index 84a69520a84..fd10aeb6100 100644 --- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/runner/CommandLineArgumentsTest.java +++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/runner/CommandLineArgumentsTest.java @@ -95,6 +95,7 @@ public class CommandLineArgumentsTest { assertThat(params.getClusters().get(0).getEndpoints().get(0).isUseSsl(), is(false)); assertThat(params.getConnectionParams().getUseCompression(), is(false)); assertThat(params.getConnectionParams().getNumPersistentConnectionsPerEndpoint(), is(16)); + assertThat(params.getConnectionParams().isEnableV3Protocol(), is(true)); assertThat(params.getFeedParams().getRoute(), is("default")); assertThat(params.getFeedParams().getDataFormat(), is(FeedParams.DataFormat.XML_UTF8)); assertThat(params.getFeedParams().getLocalQueueTimeOut(), is(180000L)); @@ -174,4 +175,32 @@ public class CommandLineArgumentsTest { assertThat(hosts, hasItem("hostValue2")); assertThat(hosts, hasItem("hostValue3")); } + + @Test + public void testDeprecatedUseV2Protocol() { + addMinimum(); + args.add("--useV2Protocol"); + CommandLineArguments arguments = CommandLineArguments.build(asArray()); + SessionParams params = arguments.createSessionParams(true /* use json */); + assertThat(params.getConnectionParams().isEnableV3Protocol(), is(false)); + } + + @Test + public void testUseV3Protocol() { + addMinimum(); + args.add("--useV3Protocol"); + CommandLineArguments arguments = CommandLineArguments.build(asArray()); + SessionParams params = arguments.createSessionParams(true /* use json */); + assertThat(params.getConnectionParams().isEnableV3Protocol(), is(true)); + } + + @Test + public void testDeprecatedUseV2ProtocolAndUseV3Protocol() { + addMinimum(); + args.add("--useV2Protocol"); + args.add("--useV3Protocol"); + CommandLineArguments arguments = CommandLineArguments.build(asArray()); + SessionParams params = arguments.createSessionParams(true /* use json */); + assertThat(params.getConnectionParams().isEnableV3Protocol(), is(true)); + } } diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java index 52d999f1a52..58457a8aa20 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java @@ -54,7 +54,7 @@ public class FeedHandler extends LoggingRequestHandler { protected final ReplyHandler feedReplyHandler; private final AtomicLong sessionId; private final Metric metric; - private static final List<Integer> serverSupportedVersions = Collections.unmodifiableList(Arrays.asList(2)); + private static final List<Integer> serverSupportedVersions = Collections.unmodifiableList(Arrays.asList(2, 3)); private final String localHostname; private final FeedHandlerV3 feedHandlerV3; diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/VersionsTestCase.java b/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/VersionsTestCase.java index ab055a54133..d6f605b0379 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/VersionsTestCase.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/vespa/http/server/VersionsTestCase.java @@ -5,6 +5,7 @@ import com.yahoo.collections.Tuple2; import com.yahoo.container.jdisc.HttpResponse; import org.junit.Test; +import java.io.ByteArrayOutputStream; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -77,6 +78,12 @@ public class VersionsTestCase { public void testTooLarge() throws Exception { Tuple2<HttpResponse, Integer> v = FeedHandler.doCheckProtocolVersion(TOO_LARGE_NUMBER); assertThat(v.first, instanceOf(ErrorHttpResponse.class)); + ByteArrayOutputStream errorMsg = new ByteArrayOutputStream(); + ErrorHttpResponse errorResponse = (ErrorHttpResponse) v.first; + errorResponse.render(errorMsg); + assertThat(errorMsg.toString(), + is("Could not parse X-Yahoo-Feed-Protocol-Versionheader of request (values: [1000000000]). " + + "Server supports protocol versions [2, 3]")); assertThat(v.second, is(-1)); } diff --git a/vespalib/src/tests/stllike/hash_test.cpp b/vespalib/src/tests/stllike/hash_test.cpp index e7fe729c1ba..017a16ee7b6 100644 --- a/vespalib/src/tests/stllike/hash_test.cpp +++ b/vespalib/src/tests/stllike/hash_test.cpp @@ -154,14 +154,14 @@ TEST("test hash set with simple type") TEST("test hash map iterator stability") { - hash_map<int, int> h; + hash_map<uint32_t, uint32_t> h; EXPECT_EQUAL(1ul, h.capacity()); for (size_t i(0); i < 100; i++) { EXPECT_TRUE(h.find(i) == h.end()); h[i] = i; EXPECT_TRUE(h.find(i) != h.end()); - int * p1 = & h.find(i)->second; - int * p2 = & h[i]; + uint32_t * p1 = & h.find(i)->second; + uint32_t * p2 = & h[i]; EXPECT_EQUAL(p1, p2); } EXPECT_EQUAL(128ul, h.capacity()); @@ -341,11 +341,11 @@ private: struct myhash { size_t operator() (const S & arg) const { return arg.hash(); } + size_t operator() (uint32_t arg) const { return arg; } }; -struct myextract { - uint32_t operator() (const S & arg) const { return arg.a(); } -}; +bool operator == (uint32_t a, const S & b) { return a == b.a(); } +bool operator == (const S & a, uint32_t b) { return a.a() == b; } TEST("test hash set find") { @@ -354,7 +354,7 @@ TEST("test hash set find") set.insert(S(i)); } EXPECT_TRUE(*set.find(S(1)) == S(1)); - hash_set<S, myhash>::iterator cit = set.find<uint32_t, myextract, vespalib::hash<uint32_t>, std::equal_to<uint32_t> >(7); + auto cit = set.find<uint32_t>(7); EXPECT_TRUE(*cit == S(7)); } diff --git a/vespalib/src/vespa/vespalib/stllike/hash_fun.h b/vespalib/src/vespa/vespalib/stllike/hash_fun.h index f708f49081e..a3401083f95 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_fun.h +++ b/vespalib/src/vespa/vespalib/stllike/hash_fun.h @@ -62,22 +62,18 @@ template<typename T> struct hash<const T *> { size_t hashValue(const char *str); size_t hashValue(const void *str, size_t sz); -template<> struct hash<const char *> { - size_t operator() (const char * arg) const { return hashValue(arg); } -}; - -template<> struct hash<vespalib::stringref> { - size_t operator() (vespalib::stringref arg) const { return hashValue(arg.data(), arg.size()); } -}; - -template<> struct hash<vespalib::string> { +struct hash_strings { size_t operator() (const vespalib::string & arg) const { return hashValue(arg.c_str()); } -}; - -template<> struct hash<std::string> { + size_t operator() (vespalib::stringref arg) const { return hashValue(arg.data(), arg.size()); } + size_t operator() (const char * arg) const { return hashValue(arg); } size_t operator() (const std::string& arg) const { return hashValue(arg.c_str()); } }; +template<> struct hash<const char *> : hash_strings { }; +template<> struct hash<vespalib::stringref> : public hash_strings { }; +template<> struct hash<vespalib::string> : hash_strings {}; +template<> struct hash<std::string> : hash_strings {}; + template<typename V> struct size { size_t operator() (const V & arg) const { return arg.size(); } }; @@ -86,6 +82,4 @@ template<typename V> struct zero { size_t operator() (const V & ) const { return 0; } }; - } // namespace vespalib - diff --git a/vespalib/src/vespa/vespalib/stllike/hash_map.h b/vespalib/src/vespa/vespalib/stllike/hash_map.h index 6d6498f8e78..34b22ba7ca3 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_map.h +++ b/vespalib/src/vespa/vespalib/stllike/hash_map.h @@ -6,7 +6,7 @@ namespace vespalib { -template< typename K, typename V, typename H = vespalib::hash<K>, typename EQ = std::equal_to<K>, typename M=hashtable_base::prime_modulator > +template< typename K, typename V, typename H = vespalib::hash<K>, typename EQ = std::equal_to<>, typename M=hashtable_base::prime_modulator > class hash_map { public: @@ -50,6 +50,16 @@ public: void erase(const_iterator it) { return erase(it->first); } iterator find(const K & key) { return _ht.find(key); } const_iterator find(const K & key) const { return _ht.find(key); } + + template< typename AltKey > + const_iterator find(const AltKey & key) const { + return _ht.template find<AltKey>(key); + } + template< typename AltKey> + iterator find(const AltKey & key) { + return _ht.template find<AltKey>(key); + } + void clear(); void resize(size_t newSize); void swap(hash_map & rhs); diff --git a/vespalib/src/vespa/vespalib/stllike/hash_map.hpp b/vespalib/src/vespa/vespalib/stllike/hash_map.hpp index b526188b8b2..74f1594965a 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_map.hpp +++ b/vespalib/src/vespa/vespalib/stllike/hash_map.hpp @@ -71,7 +71,7 @@ hash_map<K, V, H, EQ, M>::getMemoryUsed() const #define VESPALIB_HASH_MAP_INSTANTIATE_H_E(K, V, H, E) \ VESPALIB_HASH_MAP_INSTANTIATE_H_E_M(K, V, H, E, vespalib::hashtable_base::prime_modulator) -#define VESPALIB_HASH_MAP_INSTANTIATE_H(K, V, H) VESPALIB_HASH_MAP_INSTANTIATE_H_E(K, V, H, std::equal_to<K>) +#define VESPALIB_HASH_MAP_INSTANTIATE_H(K, V, H) VESPALIB_HASH_MAP_INSTANTIATE_H_E(K, V, H, std::equal_to<>) #define VESPALIB_HASH_MAP_INSTANTIATE(K, V) VESPALIB_HASH_MAP_INSTANTIATE_H(K, V, vespalib::hash<K>) diff --git a/vespalib/src/vespa/vespalib/stllike/hash_set.h b/vespalib/src/vespa/vespalib/stllike/hash_set.h index c4ccc662787..7a2db4735aa 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_set.h +++ b/vespalib/src/vespa/vespalib/stllike/hash_set.h @@ -7,7 +7,7 @@ namespace vespalib { -template< typename K, typename H = vespalib::hash<K>, typename EQ = std::equal_to<K>, typename M=hashtable_base::prime_modulator> +template< typename K, typename H = vespalib::hash<K>, typename EQ = std::equal_to<>, typename M=hashtable_base::prime_modulator> class hash_set { private: @@ -48,21 +48,11 @@ public: template <typename Func> void for_each(Func func) const { _ht.for_each(func); } - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - const_iterator find(const AltKey & key) const { return _ht.template find<AltKey, AltExtract, AltHash, AltEqual>(key); } + template< typename AltKey > + const_iterator find(const AltKey & key) const { return _ht.template find<AltKey>(key); } - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - iterator find(const AltKey & key) { return _ht.template find<AltKey, AltExtract, AltHash, AltEqual>(key); } - - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - const_iterator find(const AltKey & key, const AltExtract & altExtract) const { - return _ht.template find<AltKey, AltExtract, AltHash, AltEqual>(key, altExtract); - } - - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - iterator find(const AltKey & key, const AltExtract & altExtract) { - return _ht.template find<AltKey, AltExtract, AltHash, AltEqual>(key, altExtract); - } + template< typename AltKey> + iterator find(const AltKey & key) { return _ht.template find<AltKey>(key); } void clear(); void resize(size_t newSize); diff --git a/vespalib/src/vespa/vespalib/stllike/hash_set.hpp b/vespalib/src/vespa/vespalib/stllike/hash_set.hpp index cf6341218f1..f0427595382 100644 --- a/vespalib/src/vespa/vespalib/stllike/hash_set.hpp +++ b/vespalib/src/vespa/vespalib/stllike/hash_set.hpp @@ -84,11 +84,11 @@ hash_set<K, H, EQ, M>::insert(K &&value) { #define VESPALIB_HASH_SET_INSTANTIATE(K) \ template class vespalib::hash_set<K>; \ - template class vespalib::hashtable<K, K, vespalib::hash<K>, std::equal_to<K>, std::_Identity<K>>; \ + template class vespalib::hashtable<K, K, vespalib::hash<K>, std::equal_to<>, std::_Identity<K>>; \ template class vespalib::Array<vespalib::hash_node<K>>; #define VESPALIB_HASH_SET_INSTANTIATE_H(K, H) \ template class vespalib::hash_set<K, H>; \ - template class vespalib::hashtable<K, K, H, std::equal_to<K>, std::_Identity<K>>; \ + template class vespalib::hashtable<K, K, H, std::equal_to<>, std::_Identity<K>>; \ template class vespalib::Array<vespalib::hash_node<K>>; diff --git a/vespalib/src/vespa/vespalib/stllike/hashtable.h b/vespalib/src/vespa/vespalib/stllike/hashtable.h index 612c50ffb61..fa418cbad02 100644 --- a/vespalib/src/vespa/vespalib/stllike/hashtable.h +++ b/vespalib/src/vespa/vespalib/stllike/hashtable.h @@ -235,15 +235,12 @@ public: size_t capacity() const { return _nodes.capacity(); } size_t size() const { return _count; } bool empty() const { return _count == 0; } - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - iterator find(const AltKey & key, const AltExtract & altExtract); - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - iterator find(const AltKey & key) { return find<AltKey, AltExtract, AltHash, AltEqual>(key, AltExtract()); } + template< typename AltKey> + iterator find(const AltKey & key); iterator find(const Key & key); - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - const_iterator find(const AltKey & key, const AltExtract & altExtract) const; - template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual > - const_iterator find(const AltKey & key) const { return find<AltKey, AltExtract, AltHash, AltEqual>(key, AltExtract()); } + + template< typename AltKey> + const_iterator find(const AltKey & key) const; const_iterator find(const Key & key) const; template <typename V> insert_result insert(V && node) { @@ -285,7 +282,8 @@ protected: const Value & getByInternalIndex(size_t index) const { return _nodes[index].getValue(); } template <typename MoveHandler> void erase(MoveHandler & moveHandler, next_t h, const const_iterator & key); - next_t hash(const Key & key) const { return modulator(_hasher(key)); } + template<typename K> + next_t hash(const K & key) const { return modulator(_hasher(key)); } private: Modulator _modulator; size_t _count; diff --git a/vespalib/src/vespa/vespalib/stllike/hashtable.hpp b/vespalib/src/vespa/vespalib/stllike/hashtable.hpp index 2fbe83eb226..60c391a0b7e 100644 --- a/vespalib/src/vespa/vespalib/stllike/hashtable.hpp +++ b/vespalib/src/vespa/vespalib/stllike/hashtable.hpp @@ -95,17 +95,15 @@ hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::find(const Key & key) } template< typename Key, typename Value, typename Hash, typename Equal, typename KeyExtract, typename Modulator > -template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual> -typename hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::const_iterator -hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::find(const AltKey & key, const AltExtract & altExtract) const +template< typename AltKey> +typename hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::iterator +hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::find(const AltKey & key) { - AltHash altHasher; - next_t h = modulator(altHasher(key)); - if (_nodes[h].valid()) { - AltEqual altEqual; + next_t h = hash(key); + if (__builtin_expect(_nodes[h].valid(), true)) { do { - if (altEqual(altExtract(_keyExtractor(_nodes[h].getValue())), key)) { - return const_iterator(this, h); + if (__builtin_expect(_equal(_keyExtractor(_nodes[h].getValue()), key), true)) { + return iterator(this, h); } h = _nodes[h].getNext(); } while (h != Node::npos); @@ -114,17 +112,15 @@ hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::find(const AltKey & k } template< typename Key, typename Value, typename Hash, typename Equal, typename KeyExtract, typename Modulator > -template< typename AltKey, typename AltExtract, typename AltHash, typename AltEqual> -typename hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::iterator -hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::find(const AltKey & key, const AltExtract & altExtract) +template< typename AltKey> +typename hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::const_iterator +hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::find(const AltKey & key) const { - AltHash altHasher; - next_t h = modulator(altHasher(key)); - if (_nodes[h].valid()) { - AltEqual altEqual; + next_t h = hash(key); + if (__builtin_expect(_nodes[h].valid(), true)) { do { - if (altEqual(altExtract(_keyExtractor(_nodes[h].getValue())), key)) { - return iterator(this, h); + if (__builtin_expect(_equal(_keyExtractor(_nodes[h].getValue()), key), true)) { + return const_iterator(this, h); } h = _nodes[h].getNext(); } while (h != Node::npos); diff --git a/vespalog/src/main/java/com/yahoo/log/LogFileDb.java b/vespalog/src/main/java/com/yahoo/log/LogFileDb.java index 1ac98fc8acc..22678994346 100644 --- a/vespalog/src/main/java/com/yahoo/log/LogFileDb.java +++ b/vespalog/src/main/java/com/yahoo/log/LogFileDb.java @@ -30,13 +30,13 @@ public class LogFileDb { } private static OutputStream metaFile() throws java.io.IOException { - File dir = new File(getDefaults().underVespaHome(DBDIR)); + String fn = getDefaults().underVespaHome(DBDIR + "logfiles." + dayStamp()); + File dir = new File(fn).getParentFile(); if (!dir.exists()) { if (!dir.mkdirs()) { System.err.println("Failed creating logfiledb directory '" + dir.getPath() + "'."); } } - String fn = dir + "logfiles." + dayStamp(); Path path = Paths.get(fn); return Files.newOutputStream(path, CREATE, APPEND); } |