diff options
59 files changed, 901 insertions, 279 deletions
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackageParser.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackageParser.java index 16858808a58..f5e9a1b21a7 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackageParser.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackageParser.java @@ -140,7 +140,7 @@ public class ExportPackageParser { a backslash followed by another backslash, a single or double quote, or one of the letters b,f,n,r or t a backslash followed by u followed by four hexadecimal digits ? */ private static Pattern STRING_LITERAL_PATTERN = Pattern - .compile("\"" + "(?:[^\"\\p{Cntrl}\\\\]|\\\\[\\\\'\"bfnrt]|\\\\u[0-9a-fA-F]{4})+" + "\""); + .compile("\"(?:[^\"\\p{Cntrl}\\\\]++|\\\\[\\\\'\"bfnrt]|\\\\u[0-9a-fA-F]{4})++\""); private static Optional<String> parseStringLiteral(ParsingContext p) { return p.regexp(STRING_LITERAL_PATTERN).map(quoted -> quoted.substring(1, quoted.length() - 1)); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java index 2eff081f42d..38a6070d357 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java @@ -64,6 +64,8 @@ public class ConfigserverCluster extends AbstractConfigProducer } String myhostname = HostName.getLocalhost(); + // TODO: Server index should be in interval [1, 254] according to doc, + // however, we cannot change this id for an existing server for (int i = 0; i < configServers.length; i++) { if (zookeeperIds[i] < 0) { throw new IllegalArgumentException(String.format("Zookeeper ids cannot be negative, was %d for %s", diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index e47c00eeea3..f4c7f49a9a0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -59,6 +59,7 @@ import com.yahoo.vespa.model.container.component.chain.ProcessingHandler; import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.container.docproc.DocprocChains; import com.yahoo.vespa.model.container.http.ConnectorFactory; +import com.yahoo.vespa.model.container.http.FilterChains; import com.yahoo.vespa.model.container.http.Http; import com.yahoo.vespa.model.container.http.JettyHttpServer; import com.yahoo.vespa.model.container.http.ssl.HostedSslConnectorFactory; @@ -328,33 +329,35 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { if (httpElement != null) { cluster.setHttp(buildHttp(deployState, cluster, httpElement)); } - // If the deployment contains certificate/private key reference, setup TLS port if (deployState.tlsSecrets().isPresent()) { - boolean authorizeClient = XML.getChild(spec, "client-authorize") != null; - if (authorizeClient) { - if (deployState.tlsClientAuthority().isEmpty()) { - throw new RuntimeException("client-authorize set, but security/clients.pem is missing"); - } - } - - if(httpElement == null) { - cluster.setHttp(new Http(Collections.emptyList())); - } - if(cluster.getHttp().getHttpServer() == null) { - JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer")); - cluster.getHttp().setHttpServer(defaultHttpServer); - defaultHttpServer.addConnector(new ConnectorFactory("SearchServer", Defaults.getDefaults().vespaWebServicePort())); + addTlsPort(deployState, spec, cluster); + } + } + private void addTlsPort(DeployState deployState, Element spec, ApplicationContainerCluster cluster) { + boolean authorizeClient = XML.getChild(spec, "client-authorize") != null; + if (authorizeClient) { + if (deployState.tlsClientAuthority().isEmpty()) { + throw new RuntimeException("client-authorize set, but security/clients.pem is missing"); } - JettyHttpServer server = cluster.getHttp().getHttpServer(); - - String serverName = server.getComponentId().getName(); - HostedSslConnectorFactory connectorFactory = authorizeClient - ? new HostedSslConnectorFactory(serverName, deployState.tlsSecrets().get(), deployState.tlsClientAuthority().get()) - : new HostedSslConnectorFactory(serverName, deployState.tlsSecrets().get()); - server.addConnector(connectorFactory); } + if(cluster.getHttp() == null) { + Http http = new Http(Collections.emptyList()); + http.setFilterChains(new FilterChains(cluster)); + cluster.setHttp(http); + } + if(cluster.getHttp().getHttpServer() == null) { + JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer")); + cluster.getHttp().setHttpServer(defaultHttpServer); + defaultHttpServer.addConnector(new ConnectorFactory("SearchServer", Defaults.getDefaults().vespaWebServicePort())); + } + JettyHttpServer server = cluster.getHttp().getHttpServer(); + String serverName = server.getComponentId().getName(); + HostedSslConnectorFactory connectorFactory = authorizeClient + ? new HostedSslConnectorFactory(serverName, deployState.tlsSecrets().get(), deployState.tlsClientAuthority().get()) + : new HostedSslConnectorFactory(serverName, deployState.tlsSecrets().get()); + server.addConnector(connectorFactory); } private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) { @@ -631,10 +634,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { ClusterSpec.Id.from(cluster.getName()), deployState.getWantedNodeVespaVersion(), false); - Capacity capacity = Capacity.fromNodeCount(1, - Optional.empty(), - false, - ! deployState.getProperties().isBootstrap()); + Capacity capacity = Capacity.fromCount(1, + Optional.empty(), + false, + ! deployState.getProperties().isBootstrap()); return hostSystem.allocateHosts(clusterSpec, capacity, 1, logger).keySet().iterator().next(); } } else { diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index ed3d76353fa..b41bf96332d 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -393,7 +393,6 @@ "public com.yahoo.config.provision.Flavor with(com.yahoo.config.provision.NodeResources)", "public java.lang.String name()", "public int cost()", - "public boolean isStock()", "public boolean isConfigured()", "public com.yahoo.config.provision.NodeResources resources()", "public java.util.Optional flavorOverrides()", @@ -402,14 +401,8 @@ "public boolean hasFastDisk()", "public double getBandwidthGbps()", "public double getMinCpuCores()", - "public boolean isRetired()", "public com.yahoo.config.provision.Flavor$Type getType()", "public boolean isDocker()", - "public java.lang.String canonicalName()", - "public boolean isCanonical()", - "public java.util.List replaces()", - "public boolean satisfies(com.yahoo.config.provision.Flavor)", - "public void freeze()", "public int hashCode()", "public boolean equals(java.lang.Object)", "public java.lang.String toString()" @@ -625,8 +618,6 @@ "public com.yahoo.config.provision.NodeResources withDiskSpeed(com.yahoo.config.provision.NodeResources$DiskSpeed)", "public com.yahoo.config.provision.NodeResources subtract(com.yahoo.config.provision.NodeResources)", "public com.yahoo.config.provision.NodeResources add(com.yahoo.config.provision.NodeResources)", - "public boolean allocateByLegacyName()", - "public java.util.Optional legacyName()", "public boolean equals(java.lang.Object)", "public int hashCode()", "public java.lang.String toString()", diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java index dfa9ab7f6b8..8738fd607c9 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java @@ -71,7 +71,7 @@ public final class Capacity { /** Creates this from a desired node count: The request may be satisfied with a smaller number of nodes. */ public static Capacity fromNodeCount(int capacity) { - return fromNodeCount(capacity, Optional.empty(), false, true); + return fromCount(capacity, Optional.empty(), false, true); } /** Create a non-required, failable capacity request */ @@ -87,6 +87,8 @@ public final class Capacity { return new Capacity(nodeCount, resources, required, canFail, NodeType.tenant); } + // TODO: Remove after September 2019 + @Deprecated public static Capacity fromNodeCount(int nodeCount, Optional<String> flavor, boolean required, boolean canFail) { return new Capacity(nodeCount, flavor.map(NodeResources::fromLegacyName), required, canFail, NodeType.tenant); } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java index e814c272aa0..e511e272497 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java @@ -118,27 +118,6 @@ public class Flavor { /** Convenience, returns getType() == Type.DOCKER_CONTAINER */ public boolean isDocker() { return type == Type.DOCKER_CONTAINER; } - // TODO: Remove after August 2019 - public String canonicalName() { return name; } - - // TODO: Remove after August 2019 - public boolean satisfies(Flavor flavor) { return this.equals(flavor); } - - // TODO: Remove after August 2019 - public boolean isStock() { return false; } - - // TODO: Remove after August 2019 - public boolean isRetired() { return false; } - - // TODO: Remove after August 2019 - public boolean isCanonical() { return false; } - - // TODO: Remove after August 2019 - public List<Flavor> replaces() { return Collections.emptyList(); } - - // TODO: Remove after August 2019 - public void freeze() {} - @Override public int hashCode() { return Objects.hash(name, flavorOverrides); } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 89d497a3ab0..4469eef98cf 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -2,7 +2,6 @@ package com.yahoo.config.provision; import java.util.Objects; -import java.util.Optional; /** * The node resources required by an application cluster @@ -94,14 +93,6 @@ public class NodeResources { combine(this.diskSpeed, other.diskSpeed)); } - // TODO: Remove after August 2019 - public Optional<String> legacyName() { - return Optional.of(toString()); - } - - // TODO: Remove after August 2019 - public boolean allocateByLegacyName() { return false; } - private boolean isInterchangeableWith(NodeResources other) { if (this.diskSpeed != DiskSpeed.any && other.diskSpeed != DiskSpeed.any && this.diskSpeed != other.diskSpeed) return false; diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java index 95cf17c711c..a7ba286baa7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java @@ -145,14 +145,10 @@ public class AllocatedHostsSerializer { } else if (object.field(resourcesKey).valid()) { Inspector resources = object.field(resourcesKey); - double bandwidth = Optional.of(resources.field(bandwidthKey)) - .filter(Inspector::valid) - .map(Inspector::asDouble) - .orElse(0.3); return Optional.of(new Flavor(new NodeResources(resources.field(vcpuKey).asDouble(), resources.field(memoryKey).asDouble(), resources.field(diskKey).asDouble(), - bandwidth, + resources.field(bandwidthKey).asDouble(), diskSpeedFromSlime(resources.field(diskSpeedKey))))); } else { diff --git a/config-provisioning/src/main/resources/configdefinitions/flavors.def b/config-provisioning/src/main/resources/configdefinitions/flavors.def index 131c23054a2..59b6c852b0e 100644 --- a/config-provisioning/src/main/resources/configdefinitions/flavors.def +++ b/config-provisioning/src/main/resources/configdefinitions/flavors.def @@ -7,16 +7,10 @@ namespace=config.provisioning # If a certain flavor has no config it is not necessary to list it here to use it. flavor[].name string -# NOT USED: TODO: Remove after August 2019 -flavor[].replaces[].name string - # The monthly Total Cost of Ownership (TCO) in USD. Typically calculated as TCO divided by # the expected lifetime of the node (usually three years). flavor[].cost int default=0 -# NOT USED: TODO: Remove after August 2019 -flavor[].stock bool default=true - # The type of node: BARE_METAL, VIRTUAL_MACHINE or DOCKER_CONTAINER flavor[].environment string default="undefined" @@ -34,7 +28,3 @@ flavor[].fastDisk bool default=true # Expected network interface bandwidth available for this flavor, in Mbit/s. flavor[].bandwidth double default=0.0 - -# NOT USED: TODO: Remove after August 2019 -flavor[].retired bool default=false - diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java index cd515383950..d77206aee81 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java @@ -114,8 +114,9 @@ public class ProxyServer implements Runnable { void setMode(String modeName) { if (modeName.equals(this.mode.name())) return; - String oldMode = this.mode.name(); - switch (mode.getMode()) { + Mode oldMode = this.mode; + Mode newMode = new Mode(modeName); + switch (newMode.getMode()) { case MEMORYCACHE: configClient.shutdownSourceConnections(); configClient = new MemoryCacheConfigClient(memoryCache); @@ -129,7 +130,7 @@ public class ProxyServer implements Runnable { default: throw new IllegalArgumentException("Cannot set invalid mode '" + modeName + "'"); } - log.log(LogLevel.INFO, "Switching from '" + oldMode + "' mode to '" + modeName.toLowerCase() + "' mode"); + log.log(LogLevel.INFO, "Switched from '" + oldMode.name().toLowerCase() + "' mode to '" + getMode().name().toLowerCase() + "' mode"); } private ConfigSourceClient createClient(RpcServer rpcServer, DelayedResponses delayedResponses, diff --git a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java index 940d6d51975..8b2438e3ec2 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java +++ b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java @@ -2,6 +2,9 @@ package com.yahoo.search.yql; import com.google.common.annotations.Beta; +import com.google.inject.Inject; +import com.yahoo.language.Linguistics; +import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; @@ -18,6 +21,8 @@ import com.yahoo.yolean.chain.After; import com.yahoo.yolean.chain.Before; import com.yahoo.yolean.chain.Provides; +import java.util.logging.Logger; + /** * Minimal combinator for YQL+ syntax and heuristically parsed user queries. * @@ -35,44 +40,62 @@ public class MinimalQueryInserter extends Searcher { private static final CompoundName MAX_HITS = new CompoundName("maxHits"); private static final CompoundName MAX_OFFSET = new CompoundName("maxOffset"); + private static Logger log = Logger.getLogger(MinimalQueryInserter.class.getName()); - @Override - public Result search(Query query, Execution execution) { - if (query.properties().get(YQL) == null) return execution.search(query); + @Inject + public MinimalQueryInserter(Linguistics linguistics) { + // Warmup is needed to avoid a large 400ms init cost during first execution of yql code. + warmup(linguistics); + } + public MinimalQueryInserter() { + this(new SimpleLinguistics()); + } + static boolean warmup() { + return warmup(new SimpleLinguistics()); + } + private static boolean warmup(Linguistics linguistics) { + Query query = new Query("search/?yql=select%20*%20from%20sources%20where%20title%20contains%20'xyz';"); + Result result = insertQuery(query, new ParserEnvironment().setLinguistics(linguistics)); + if (result != null) { + log.warning("Warmup code trigger an error. Error = " + result.toString()); + return false; + } + if ( ! "select * from sources where title contains \"xyz\";".equals(query.yqlRepresentation())) { + log.warning("Warmup code generated unexpected yql: " + query.yqlRepresentation()); + return false; + } + return true; + } - ParserEnvironment env = ParserEnvironment.fromExecutionContext(execution.context()); + private static Result insertQuery(Query query, ParserEnvironment env) { YqlParser parser = (YqlParser) ParserFactory.newInstance(Query.Type.YQL, env); parser.setQueryParser(false); parser.setUserQuery(query); QueryTree newTree; try { - newTree = parser.parse(Parsable.fromQueryModel(query.getModel()) - .setQuery(query.properties().getString(YQL))); + Parsable parsable = Parsable.fromQueryModel(query.getModel()).setQuery(query.properties().getString(YQL)); + newTree = parser.parse(parsable); } catch (RuntimeException e) { - return new Result(query, ErrorMessage.createInvalidQueryParameter( - "Could not instantiate query from YQL", e)); + return new Result(query, ErrorMessage.createInvalidQueryParameter("Could not instantiate query from YQL", e)); } if (parser.getOffset() != null) { int maxHits = query.properties().getInteger(MAX_HITS); int maxOffset = query.properties().getInteger(MAX_OFFSET); if (parser.getOffset() > maxOffset) { return new Result(query, ErrorMessage.createInvalidQueryParameter("Requested offset " + parser.getOffset() - + ", but the max offset allowed is " + - maxOffset + ".")); + + ", but the max offset allowed is " + maxOffset + ".")); } if (parser.getHits() > maxHits) { return new Result(query, ErrorMessage.createInvalidQueryParameter("Requested " + parser.getHits() - + " hits returned, but max hits allowed is " - + maxHits + ".")); - + + " hits returned, but max hits allowed is " + maxHits + ".")); } } query.getModel().getQueryTree().setRoot(newTree.getRoot()); query.getPresentation().getSummaryFields().addAll(parser.getYqlSummaryFields()); for (VespaGroupingStep step : parser.getGroupingSteps()) { GroupingRequest.newInstance(query) - .setRootOperation(step.getOperation()) - .continuations().addAll(step.continuations()); + .setRootOperation(step.getOperation()) + .continuations().addAll(step.continuations()); } if (parser.getYqlSources().size() == 0) { query.getModel().getSources().clear(); @@ -90,7 +113,15 @@ public class MinimalQueryInserter extends Searcher { query.getRanking().setSorting(parser.getSorting()); } query.trace("YQL+ query parsed", true, 2); - return execution.search(query); + return null; + } + + @Override + public Result search(Query query, Execution execution) { + if (query.properties().get(YQL) == null) return execution.search(query); + + Result result = insertQuery(query, ParserEnvironment.fromExecutionContext(execution.context())); + return (result == null) ? execution.search(query) : result; } } diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/TracingOptions.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/TracingOptions.java new file mode 100644 index 00000000000..3c96b00d628 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/TracingOptions.java @@ -0,0 +1,83 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors; + +import com.yahoo.vespa.streamingvisitors.tracing.LoggingTraceExporter; +import com.yahoo.vespa.streamingvisitors.tracing.MaxSamplesPerPeriod; +import com.yahoo.vespa.streamingvisitors.tracing.MonotonicNanoClock; +import com.yahoo.vespa.streamingvisitors.tracing.ProbabilisticSampleRate; +import com.yahoo.vespa.streamingvisitors.tracing.SamplingStrategy; +import com.yahoo.vespa.streamingvisitors.tracing.SamplingTraceExporter; +import com.yahoo.vespa.streamingvisitors.tracing.TraceExporter; + +import java.util.concurrent.TimeUnit; + +/** + * Encapsulates all trace-related components and options used by the streaming search Searcher. + * + * Provides a DEFAULT static instance which has the following characteristics: + * - Approximately 1 query every 2 seconds is traced + * - Trace level is set to 7 for traced queries + * - Only emits traces for queries that have timed out and where the elapsed time is at least 5x + * of the timeout specified in the query itself + * - Emits traces to the Vespa log + * - Only 1 trace every 10 seconds may be emitted to the log + */ +public class TracingOptions { + + private final SamplingStrategy samplingStrategy; + private final TraceExporter traceExporter; + private final MonotonicNanoClock clock; + private final int traceLevelOverride; + private final double traceTimeoutMultiplierThreshold; + + /** + * @param samplingStrategy used for choosing if a query should have its trace level implicitly altered. + * @param traceExporter used for emitting a visitor session trace to someplace it may be debugged later. + * @param clock monotonic clock used for relative time tracking. + * @param traceLevelOverride if a query is trace-sampled, its traceLevel will be set to this value + * @param traceTimeoutMultiplierThreshold only export traces if the elapsed time is greater than the query timeout * this value + */ + public TracingOptions(SamplingStrategy samplingStrategy, TraceExporter traceExporter, + MonotonicNanoClock clock, int traceLevelOverride, double traceTimeoutMultiplierThreshold) + { + this.samplingStrategy = samplingStrategy; + this.traceExporter = traceExporter; + this.clock = clock; + this.traceLevelOverride = traceLevelOverride; + this.traceTimeoutMultiplierThreshold = traceTimeoutMultiplierThreshold; + } + + public static final TracingOptions DEFAULT; + public static final int DEFAULT_TRACE_LEVEL_OVERRIDE = 7; // TODO determine appropriate trace level + // Traces won't be exported unless the query has timed out with a duration that is > timeout * multiplier + public static final double TRACE_TIMEOUT_MULTIPLIER_THRESHOLD = 5.0; + + static { + SamplingStrategy queryTraceSampler = ProbabilisticSampleRate.withSystemDefaults(0.5); + SamplingStrategy logExportSampler = MaxSamplesPerPeriod.withSteadyClock(TimeUnit.SECONDS.toNanos(10), 1); + TraceExporter traceExporter = new SamplingTraceExporter(new LoggingTraceExporter(), logExportSampler); + DEFAULT = new TracingOptions(queryTraceSampler, traceExporter, System::nanoTime, + DEFAULT_TRACE_LEVEL_OVERRIDE, TRACE_TIMEOUT_MULTIPLIER_THRESHOLD); + } + + public SamplingStrategy getSamplingStrategy() { + return samplingStrategy; + } + + public TraceExporter getTraceExporter() { + return traceExporter; + } + + public MonotonicNanoClock getClock() { + return clock; + } + + public int getTraceLevelOverride() { + return traceLevelOverride; + } + + public double getTraceTimeoutMultiplierThreshold() { + return traceTimeoutMultiplierThreshold; + } + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java index 827fcb885df..c1217f7acc2 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.streamingvisitors; import com.yahoo.document.DocumentId; -import com.yahoo.document.idstring.IdString; import com.yahoo.document.select.parser.ParseException; import com.yahoo.document.select.parser.TokenMgrException; import com.yahoo.fs4.DocsumPacket; @@ -26,11 +25,13 @@ import com.yahoo.searchlib.aggregation.Grouping; import com.yahoo.vdslib.DocumentSummary; import com.yahoo.vdslib.SearchResult; import com.yahoo.vdslib.VisitorStatistics; +import com.yahoo.vespa.streamingvisitors.tracing.TraceDescription; import java.io.IOException; import java.math.BigInteger; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.logging.Logger; /** @@ -49,8 +50,10 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { private static final CompoundName streamingSelection=new CompoundName("streaming.selection"); public static final String STREAMING_STATISTICS = "streaming.statistics"; - private VisitorFactory visitorFactory; + private final VisitorFactory visitorFactory; + private final TracingOptions tracingOptions; private static final Logger log = Logger.getLogger(VdsStreamingSearcher.class.getName()); + private Route route; /** The configId used to access the searchcluster. */ private String searchClusterConfigId = null; @@ -73,26 +76,105 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { private static class VdsVisitorFactory implements VisitorFactory { @Override - public Visitor createVisitor(Query query, String searchCluster, Route route, String documentType) { - return new VdsVisitor(query, searchCluster, route, documentType); + public Visitor createVisitor(Query query, String searchCluster, Route route, String documentType, int traceLevelOverride) { + return new VdsVisitor(query, searchCluster, route, documentType, traceLevelOverride); } } public VdsStreamingSearcher() { - visitorFactory = new VdsVisitorFactory(); + this(new VdsVisitorFactory()); } public VdsStreamingSearcher(VisitorFactory visitorFactory) { this.visitorFactory = visitorFactory; + tracingOptions = TracingOptions.DEFAULT; + } + + public VdsStreamingSearcher(VisitorFactory visitorFactory, TracingOptions tracingOptions) { + this.visitorFactory = visitorFactory; + this.tracingOptions = tracingOptions; } @Override protected void doPartialFill(Result result, String summaryClass) { } + private double durationInMillisFromNanoTime(long startTimeNanos) { + return (tracingOptions.getClock().nanoTimeNow() - startTimeNanos) / (double)TimeUnit.MILLISECONDS.toNanos(1); + } + + private boolean timeoutBadEnoughToBeReported(Query query, double durationMillis) { + return (durationMillis > (query.getTimeout() * tracingOptions.getTraceTimeoutMultiplierThreshold())); + } + + private static boolean queryIsLocationConstrained(Query query) { + return ((query.properties().getString(streamingUserid) != null) || + (query.properties().getString(streamingGroupname) != null)); + } + + private static int documentSelectionQueryParameterCount(Query query) { + int paramCount = 0; + if (query.properties().getString(streamingUserid) != null) { + paramCount++; + } + if (query.properties().getString(streamingGroupname) != null) { + paramCount++; + } + if (query.properties().getString(streamingSelection) != null) { + paramCount++; + } + return paramCount; + } + + private boolean shouldTraceQuery(Query query) { + // Only trace for explicit bucket subset queries, as otherwise we'd get a trace entry for every superbucket in the system. + return (queryIsLocationConstrained(query) && + ((query.getTraceLevel() > 0) || tracingOptions.getSamplingStrategy().shouldSample())); + } + + private int inferEffectiveQueryTraceLevel(Query query) { + return ((query.getTraceLevel() == 0) && shouldTraceQuery(query)) // Honor query's explicit trace level if present. + ? tracingOptions.getTraceLevelOverride() + : query.getTraceLevel(); + } + @Override public Result doSearch2(Query query, Execution execution) { - // TODO refactor this method into smaller methods, it's hard to see the actual code + initializeMissingQueryFields(query); + if (documentSelectionQueryParameterCount(query) != 1) { + return new Result(query, ErrorMessage.createBackendCommunicationError("Streaming search needs one and " + + "only one of these query parameters to be set: streaming.userid, streaming.groupname, " + + "streaming.selection")); + } + query.trace("Routing to search cluster " + getSearchClusterConfigId() + " and document type " + documentType, 4); + long timeStartedNanos = tracingOptions.getClock().nanoTimeNow(); + int effectiveTraceLevel = inferEffectiveQueryTraceLevel(query); + + Visitor visitor = visitorFactory.createVisitor(query, getSearchClusterConfigId(), route, documentType, effectiveTraceLevel); + try { + visitor.doSearch(); + } catch (ParseException e) { + return new Result(query, ErrorMessage.createBackendCommunicationError( + "Failed to parse document selection string: " + e.getMessage() + "'.")); + } catch (TokenMgrException e) { + return new Result(query, ErrorMessage.createBackendCommunicationError( + "Failed to tokenize document selection string: " + e.getMessage() + "'.")); + } catch (TimeoutException e) { + double elapsedMillis = durationInMillisFromNanoTime(timeStartedNanos); + if ((effectiveTraceLevel > 0) && timeoutBadEnoughToBeReported(query, elapsedMillis)) { + tracingOptions.getTraceExporter().maybeExport(() -> new TraceDescription(visitor.getTrace(), + String.format("Trace of %s which timed out after %.3g seconds", + query.toString(), elapsedMillis / 1000.0))); + } + return new Result(query, ErrorMessage.createTimeout(e.getMessage())); + } catch (InterruptedException|IllegalArgumentException e) { + return new Result(query, ErrorMessage.createBackendCommunicationError(e.getMessage())); + } + + return buildResultFromCompletedVisitor(query, visitor); + } + + private void initializeMissingQueryFields(Query query) { lazyTrace(query, 7, "Routing to storage cluster ", getStorageClusterRouteSpec()); if (route == null) { @@ -119,32 +201,9 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { lazyTrace(query, 8, "doSearch2(): sort specification=", query .getRanking().getSorting() == null ? null : query.getRanking() .getSorting().fieldOrders()); + } - int documentSelectionQueryParameterCount = 0; - if (query.properties().getString(streamingUserid) != null) documentSelectionQueryParameterCount++; - if (query.properties().getString(streamingGroupname) != null) documentSelectionQueryParameterCount++; - if (query.properties().getString(streamingSelection) != null) documentSelectionQueryParameterCount++; - if (documentSelectionQueryParameterCount != 1) { - return new Result(query, ErrorMessage.createBackendCommunicationError("Streaming search needs one and " + - "only one of these query parameters to be set: streaming.userid, streaming.groupname, " + - "streaming.selection")); - } - query.trace("Routing to search cluster " + getSearchClusterConfigId() + " and document type " + documentType, 4); - Visitor visitor = visitorFactory.createVisitor(query, getSearchClusterConfigId(), route, documentType); - try { - visitor.doSearch(); - } catch (ParseException e) { - return new Result(query, ErrorMessage.createBackendCommunicationError( - "Failed to parse document selection string: " + e.getMessage() + "'.")); - } catch (TokenMgrException e) { - return new Result(query, ErrorMessage.createBackendCommunicationError( - "Failed to tokenize document selection string: " + e.getMessage() + "'.")); - } catch (TimeoutException e) { - return new Result(query, ErrorMessage.createTimeout(e.getMessage())); - } catch (InterruptedException|IllegalArgumentException e) { - return new Result(query, ErrorMessage.createBackendCommunicationError(e.getMessage())); - } - + private Result buildResultFromCompletedVisitor(Query query, Visitor visitor) { lazyTrace(query, 8, "offset=", query.getOffset(), ", hits=", query.getHits()); Result result = new Result(query); @@ -176,7 +235,6 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { DocumentSummary.Summary summary = summaryMap.get(hit.getDocId()); if (summary != null) { DocsumPacket dp = new DocsumPacket(summary.getSummary()); - //log.log(LogLevel.SPAM, "DocsumPacket: " + dp); summaryPackets[index] = dp; } else { return new Result(query, ErrorMessage.createBackendCommunicationError( @@ -213,7 +271,7 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { return new Result(query, ErrorMessage.createBackendCommunicationError("Error filling hits with summary fields")); } - if (skippedHits==0) { + if (skippedHits == 0) { query.trace("All hits have been filled",4); // TODO: cache results or result.analyzeHits(); ? } else { lazyTrace(query, 8, "Skipping some hits for query: ", result.getQuery()); @@ -221,7 +279,7 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { lazyTrace(query, 8, "Returning result ", result); - if ( skippedHits>0 ) { + if (skippedHits > 0) { getLogger().info("skipping " + skippedHits + " hits for query: " + result.getQuery()); result.hits().addError(com.yahoo.search.result.ErrorMessage.createTimeout("Missing hit summary data for " + skippedHits + " hits")); } diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java index 628c24fffd1..795b62663d5 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java @@ -19,6 +19,7 @@ import com.yahoo.documentapi.messagebus.protocol.SearchResultMessage; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.log.LogLevel; import com.yahoo.messagebus.Message; +import com.yahoo.messagebus.Trace; import com.yahoo.messagebus.routing.Route; import com.yahoo.prelude.fastsearch.TimeoutException; import com.yahoo.processing.request.CompoundName; @@ -72,6 +73,8 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { private final Map<Integer, Grouping> groupingMap = new ConcurrentHashMap<>(); private Query query = null; private VisitorSessionFactory visitorSessionFactory; + private final int traceLevelOverride; + private Trace sessionTrace; public interface VisitorSessionFactory { VisitorSession createVisitorSession(VisitorParameters params) throws ParseException; @@ -123,20 +126,22 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { } } - public VdsVisitor(Query query, String searchCluster, Route route, String documentType) { - this(query, searchCluster, route, documentType, MessageBusVisitorSessionFactory.sharedInstance()); + public VdsVisitor(Query query, String searchCluster, Route route, String documentType, int traceLevelOverride) { + this(query, searchCluster, route, documentType, MessageBusVisitorSessionFactory.sharedInstance(), traceLevelOverride); } public VdsVisitor(Query query, String searchCluster, Route route, - String documentType, VisitorSessionFactory visitorSessionFactory) + String documentType, VisitorSessionFactory visitorSessionFactory, + int traceLevelOverride) { this.query = query; this.visitorSessionFactory = visitorSessionFactory; + this.traceLevelOverride = traceLevelOverride; setVisitorParameters(searchCluster, route, documentType); } - private static int inferSessionTraceLevel(Query query) { - int implicitLevel = 0; + private int inferSessionTraceLevel(Query query) { + int implicitLevel = traceLevelOverride; if (log.isLoggable(LogLevel.SPAM)) { implicitLevel = 9; } else if (log.isLoggable(LogLevel.DEBUG)) { @@ -331,11 +336,11 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { } } finally { session.destroy(); - log.log(LogLevel.DEBUG, () -> session.getTrace().toString()); + sessionTrace = session.getTrace(); + log.log(LogLevel.DEBUG, () -> sessionTrace.toString()); + query.trace(sessionTrace.toString(), false, 9); } - query.trace(session.getTrace().toString(), false, 9); - if (params.getControlHandler().getResult().code == VisitorControlHandler.CompletionCode.SUCCESS) { if (log.isLoggable(LogLevel.DEBUG)) { log.log(LogLevel.DEBUG, "VdsVisitor completed successfully for " + query + " with selection " + params.getDocumentSelection()); @@ -368,6 +373,11 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { ack(token); } + @Override + public Trace getTrace() { + return sessionTrace; + } + public void onQueryResult(SearchResult sr, DocumentSummary summary) { handleSearchResult(sr); handleSummary(summary); diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/Visitor.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/Visitor.java index 45a525896e9..e8b83495c69 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/Visitor.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/Visitor.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.streamingvisitors; import com.yahoo.document.select.parser.ParseException; +import com.yahoo.messagebus.Trace; import com.yahoo.prelude.fastsearch.TimeoutException; import com.yahoo.searchlib.aggregation.Grouping; import com.yahoo.vdslib.DocumentSummary; @@ -29,4 +30,7 @@ interface Visitor { int getTotalHitCount(); List<Grouping> getGroupings(); + + Trace getTrace(); + } diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VisitorFactory.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VisitorFactory.java index 9762d05bf45..7ce323a2f2b 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VisitorFactory.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VisitorFactory.java @@ -10,5 +10,7 @@ import com.yahoo.search.Query; * @author <a href="mailto:ulf@yahoo-inc.com">Ulf Carlin</a> */ interface VisitorFactory { - public Visitor createVisitor(Query query, String searchCluster, Route route, String documentType); + + Visitor createVisitor(Query query, String searchCluster, Route route, String documentType, int traceLevelOverride); + } diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/LoggingTraceExporter.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/LoggingTraceExporter.java new file mode 100644 index 00000000000..0aaf301e071 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/LoggingTraceExporter.java @@ -0,0 +1,25 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import com.yahoo.log.LogLevel; + +import java.util.function.Supplier; +import java.util.logging.Logger; + +/** + * Trace exporter which dumps traces and their description as warning-entries in the Vespa log. + */ +public class LoggingTraceExporter implements TraceExporter { + + private static final Logger log = Logger.getLogger(LoggingTraceExporter.class.getName()); + + @Override + public void maybeExport(Supplier<TraceDescription> traceDescriptionSupplier) { + var traceDescription = traceDescriptionSupplier.get(); + if (traceDescription.getTrace() != null) { + log.log(LogLevel.WARNING, String.format("%s: %s", traceDescription.getDescription(), + traceDescription.getTrace().toString())); + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/MaxSamplesPerPeriod.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/MaxSamplesPerPeriod.java new file mode 100644 index 00000000000..83290985007 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/MaxSamplesPerPeriod.java @@ -0,0 +1,46 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +/** + * Very basic sampling strategy which allows for sampling N requests within a fixed + * time window. No attempt is made to distribute the samples evenly within the time + * period; this is on a first-come, first-serve basis. + */ +public class MaxSamplesPerPeriod implements SamplingStrategy { + + private final MonotonicNanoClock nanoClock; + private final long maxSamplesPerPeriod; + private final long periodLengthInNanos; + private long currentSamplingPeriod = 0; + private long samplesInCurrentPeriod = 0; + + public MaxSamplesPerPeriod(MonotonicNanoClock nanoClock, long periodLengthInNanos, long maxSamplesPerPeriod) { + this.nanoClock = nanoClock; + this.periodLengthInNanos = periodLengthInNanos; + this.maxSamplesPerPeriod = maxSamplesPerPeriod; + } + + public static MaxSamplesPerPeriod withSteadyClock(long periodLengthInNanos, long maxSamplesPerPeriod) { + // We make a reasonable assumption that System.nanoTime uses the underlying steady clock. + return new MaxSamplesPerPeriod(System::nanoTime, periodLengthInNanos, maxSamplesPerPeriod); + } + + @Override + public boolean shouldSample() { + long now = nanoClock.nanoTimeNow(); + long period = now / periodLengthInNanos; + synchronized (this) { + if (period != currentSamplingPeriod) { + currentSamplingPeriod = period; + samplesInCurrentPeriod = 1; + return true; + } + if (samplesInCurrentPeriod >= maxSamplesPerPeriod) { + return false; + } + ++samplesInCurrentPeriod; + return true; + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/MonotonicNanoClock.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/MonotonicNanoClock.java new file mode 100644 index 00000000000..9c9815c2dbc --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/MonotonicNanoClock.java @@ -0,0 +1,15 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +/** + * Clock which returns a monotonically increasing timestamp from an undefined epoch. + * The epoch is guaranteed to be stable within a single JVM execution, but not across + * processes. Should therefore only be used for relative duration tracking, not absolute + * wall clock time events. + */ +@FunctionalInterface +public interface MonotonicNanoClock { + + long nanoTimeNow(); + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/ProbabilisticSampleRate.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/ProbabilisticSampleRate.java new file mode 100644 index 00000000000..bc6e91afe17 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/ProbabilisticSampleRate.java @@ -0,0 +1,52 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; + +/** + * Simple implementation of OpenCensus algorithm for probabilistic rate limiting as outlined in + * https://github.com/census-instrumentation/opencensus-specs/blob/master/trace/Sampling.md + */ +public class ProbabilisticSampleRate implements SamplingStrategy { + + private final MonotonicNanoClock nanoClock; + private final Supplier<Random> randomSupplier; + private final double desiredSamplesPerSec; + private final AtomicLong lastSampledAtNanoTime = new AtomicLong(0); + + public ProbabilisticSampleRate(MonotonicNanoClock nanoClock, + Supplier<Random> randomSupplier, + double desiredSamplesPerSec) + { + this.nanoClock = nanoClock; + this.randomSupplier = randomSupplier; + this.desiredSamplesPerSec = desiredSamplesPerSec; + } + + public static ProbabilisticSampleRate withSystemDefaults(double desiredSamplesPerSec) { + return new ProbabilisticSampleRate(System::nanoTime, ThreadLocalRandom::current, desiredSamplesPerSec); + } + + @Override + public boolean shouldSample() { + // This load might race with the store below, causing multiple threads to get a sample + // since the new timestamp has not been written yet, but it is extremely unlikely and + // the consequences are not severe since this is a probabilistic sampler that does not + // provide hard lower or upper bounds. + long lastSampledAt = lastSampledAtNanoTime.get(); // TODO getPlain? No transitive visibility requirements + long now = nanoClock.nanoTimeNow(); + double secsSinceLastSample = (now - lastSampledAt) / 1_000_000_000.0; + // As the time elapsed since last sample increases, so does the probability of a new sample + // being selected. + double sampleProb = Math.min(secsSinceLastSample * desiredSamplesPerSec, 1.0); + if (randomSupplier.get().nextDouble() < sampleProb) { + lastSampledAtNanoTime.set(now); // TODO setPlain? No transitive visibility requirements + return true; + } else { + return false; + } + } +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingStrategy.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingStrategy.java new file mode 100644 index 00000000000..3ce7864c081 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingStrategy.java @@ -0,0 +1,16 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +/** + * A sampling strategy makes the high-level decision of whether or not a query + * should be traced. + * + * Callers should be able to expect that calling shouldSample() is a cheap operation + * with little or no underlying locking. This in turn means that the sampling strategy + * may be consulted for each query with minimal overhead. + */ +public interface SamplingStrategy { + + boolean shouldSample(); + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingTraceExporter.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingTraceExporter.java new file mode 100644 index 00000000000..b18e8d78266 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingTraceExporter.java @@ -0,0 +1,26 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import java.util.function.Supplier; + +/** + * Trace exporter which only exports a subset of traces as decided by the provided sampling strategy. + */ +public class SamplingTraceExporter implements TraceExporter { + + private final TraceExporter wrappedExporter; + private final SamplingStrategy samplingStrategy; + + public SamplingTraceExporter(TraceExporter wrappedExporter, SamplingStrategy samplingStrategy) { + this.wrappedExporter = wrappedExporter; + this.samplingStrategy = samplingStrategy; + } + + @Override + public void maybeExport(Supplier<TraceDescription> traceDescriptionSupplier) { + if (samplingStrategy.shouldSample()) { + wrappedExporter.maybeExport(traceDescriptionSupplier); + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/TraceDescription.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/TraceDescription.java new file mode 100644 index 00000000000..74daef05756 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/TraceDescription.java @@ -0,0 +1,29 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import com.yahoo.messagebus.Trace; + +/** + * High-level description of a trace and the actual trace itself. Used to provide more + * information to logs etc than just the trace tree. The description will usually contain + * context for the trace, such as the orginal query string, desired timeout etc. + */ +public class TraceDescription { + + private final Trace trace; + private final String description; + + public TraceDescription(Trace trace, String description) { + this.trace = trace; + this.description = description; + } + + public Trace getTrace() { + return trace; + } + + public String getDescription() { + return description; + } + +} diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/TraceExporter.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/TraceExporter.java new file mode 100644 index 00000000000..0f55408e45b --- /dev/null +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/tracing/TraceExporter.java @@ -0,0 +1,15 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import java.util.function.Supplier; + +/** + * Potentially exports a trace to an underlying consumer. "Potentially" here means + * that the exporter may itself sample or otherwise limit which queries are actually + * exported. + */ +public interface TraceExporter { + + void maybeExport(Supplier<TraceDescription> traceDescriptionSupplier); + +} diff --git a/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java index 22c17f556c8..d832ba52ceb 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java @@ -341,6 +341,11 @@ public class MinimalQueryInserterTestCase { query.yqlRepresentation()); } + @Test + public void verifyThatWarmupIsSane() { + assertTrue(MinimalQueryInserter.warmup()); + } + private static void assertGrouping(String expected, Query query) { List<String> actual = new ArrayList<>(); diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java index 9dd6aae9e7b..c6ab1a8454f 100644 --- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java @@ -3,11 +3,11 @@ package com.yahoo.vespa.streamingvisitors; import com.yahoo.config.subscription.ConfigGetter; import com.yahoo.document.select.parser.TokenMgrException; +import com.yahoo.messagebus.Trace; import com.yahoo.messagebus.routing.Route; import com.yahoo.prelude.fastsearch.ClusterParams; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.document.select.parser.ParseException; -import com.yahoo.fs4.QueryPacket; import com.yahoo.prelude.fastsearch.SummaryParameters; import com.yahoo.prelude.fastsearch.TimeoutException; import com.yahoo.search.Query; @@ -18,21 +18,31 @@ import com.yahoo.searchlib.aggregation.Grouping; import com.yahoo.vdslib.DocumentSummary; import com.yahoo.vdslib.SearchResult; import com.yahoo.vdslib.VisitorStatistics; +import com.yahoo.vespa.streamingvisitors.tracing.MockUtils; +import com.yahoo.vespa.streamingvisitors.tracing.MonotonicNanoClock; +import com.yahoo.vespa.streamingvisitors.tracing.SamplingStrategy; +import com.yahoo.vespa.streamingvisitors.tracing.TraceExporter; import org.junit.Test; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** - * @author <a href="mailto:ulf@yahoo-inc.com">Ulf Carlin</a> + * @author Ulf Carlin */ public class VdsStreamingSearcherTestCase { public static final String USERDOC_ID_PREFIX = "id:namespace:mytype:n=1:userspecific"; @@ -47,12 +57,14 @@ public class VdsStreamingSearcherTestCase { private final List<SearchResult.Hit> hits = new ArrayList<>(); private final Map<String, DocumentSummary.Summary> summaryMap = new HashMap<>(); private final List<Grouping> groupings = new ArrayList<>(); + int traceLevelOverride; - MockVisitor(Query query, String searchCluster, Route route, String documentType) { + MockVisitor(Query query, String searchCluster, Route route, String documentType, int traceLevelOverride) { this.query = query; this.searchCluster = searchCluster; this.route = route; this.documentType = documentType; + this.traceLevelOverride = traceLevelOverride; } @Override @@ -126,12 +138,21 @@ public class VdsStreamingSearcherTestCase { public List<Grouping> getGroupings() { return groupings; } + + @Override + public Trace getTrace() { + return new Trace(); + } } private static class MockVisitorFactory implements VisitorFactory { + + public MockVisitor lastCreatedVisitor; + @Override - public Visitor createVisitor(Query query, String searchCluster, Route route, String documentType) { - return new MockVisitor(query, searchCluster, route, documentType); + public Visitor createVisitor(Query query, String searchCluster, Route route, String documentType, int traceLevelOverride) { + lastCreatedVisitor = new MockVisitor(query, searchCluster, route, documentType, traceLevelOverride); + return lastCreatedVisitor; } } @@ -263,4 +284,73 @@ public class VdsStreamingSearcherTestCase { assertFalse(VdsStreamingSearcher.verifyDocId(badId, group1Query, false)); } + private static class TraceFixture { + SamplingStrategy sampler = mock(SamplingStrategy.class); + TraceExporter exporter = mock(TraceExporter.class); + MonotonicNanoClock clock; + TracingOptions options; + + MockVisitorFactory factory; + VdsStreamingSearcher searcher; + + private TraceFixture(Long firstTimestamp, Long... additionalTimestamps) { + clock = MockUtils.mockedClockReturning(firstTimestamp, additionalTimestamps); + options = new TracingOptions(sampler, exporter, clock, 8, 2.0); + factory = new MockVisitorFactory(); + searcher = new VdsStreamingSearcher(factory, options); + } + + private TraceFixture() { + this(TimeUnit.SECONDS.toNanos(1), TimeUnit.SECONDS.toNanos(10)); + } + + static TraceFixture withSampledTrace(boolean shouldTrace) { + var f = new TraceFixture(); + when(f.sampler.shouldSample()).thenReturn(shouldTrace); + return f; + } + + static TraceFixture withTracingAndClockSampledAt(long t1ms, long t2ms) { + var f = new TraceFixture(TimeUnit.MILLISECONDS.toNanos(t1ms), TimeUnit.MILLISECONDS.toNanos(t2ms)); + when(f.sampler.shouldSample()).thenReturn(true); + return f; + } + } + + @Test + public void trace_level_set_if_sampling_strategy_returns_true() { + var f = TraceFixture.withSampledTrace(true); + executeQuery(f.searcher, new Query("/?streaming.userid=1&query=timeoutexception")); + + assertNotNull(f.factory.lastCreatedVisitor); + assertEquals(f.factory.lastCreatedVisitor.traceLevelOverride, 8); + } + + @Test + public void trace_level_not_set_if_sampling_strategy_returns_false() { + var f = TraceFixture.withSampledTrace(false); + executeQuery(f.searcher, new Query("/?streaming.userid=1&query=timeoutexception")); + + assertNotNull(f.factory.lastCreatedVisitor); + assertEquals(f.factory.lastCreatedVisitor.traceLevelOverride, 0); + } + + @Test + public void trace_is_exported_if_timed_out_beyond_threshold() { + // Default mock timeout threshold is 2x timeout + var f = TraceFixture.withTracingAndClockSampledAt(1000, 3001); + executeQuery(f.searcher, new Query("/?streaming.userid=1&query=timeoutexception&timeout=1.0")); + + verify(f.exporter, times(1)).maybeExport(any()); + } + + @Test + public void trace_is_not_exported_if_timed_out_less_than_threshold() { + // Default mock timeout threshold is 2x timeout + var f = TraceFixture.withTracingAndClockSampledAt(1000, 2999); + executeQuery(f.searcher, new Query("/?streaming.userid=1&query=timeoutexception&timeout=1.0")); + + verify(f.exporter, times(0)).maybeExport(any()); + } + } diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java index 9119a5cd0f1..7841b6f715c 100644 --- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java @@ -363,7 +363,7 @@ public class VdsVisitorTestCase { } private void verifyVisitorOk(MockVisitorSessionFactory factory, QueryArguments qa, Route route, String searchCluster) throws Exception { - VdsVisitor visitor = new VdsVisitor(buildQuery(qa), searchCluster, route, "mytype", factory); + VdsVisitor visitor = new VdsVisitor(buildQuery(qa), searchCluster, route, "mytype", factory, 0); visitor.doSearch(); verifyVisitorParameters(factory.getParams(), qa, searchCluster, "mytype", route); supplyResults(visitor); @@ -371,7 +371,7 @@ public class VdsVisitorTestCase { } private void verifyVisitorFails(MockVisitorSessionFactory factory, QueryArguments qa, Route route, String searchCluster) throws Exception { - VdsVisitor visitor = new VdsVisitor(buildQuery(qa), searchCluster, route, "mytype", factory); + VdsVisitor visitor = new VdsVisitor(buildQuery(qa), searchCluster, route, "mytype", factory, 0); try { visitor.doSearch(); assertTrue("Visitor did not fail", false); diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/MaxSamplesPerPeriodTest.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/MaxSamplesPerPeriodTest.java new file mode 100644 index 00000000000..0ab42cfdcfc --- /dev/null +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/MaxSamplesPerPeriodTest.java @@ -0,0 +1,37 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class MaxSamplesPerPeriodTest { + + @Test + public void first_sample_in_period_returns_true() { + var clock = MockUtils.mockedClockReturning(1000L); + var sampler = new MaxSamplesPerPeriod(clock, 1000L, 1L); + assertTrue(sampler.shouldSample()); + } + + @Test + public void samples_exceeding_period_count_return_false() { + var clock = MockUtils.mockedClockReturning(1000L, 1100L, 1200L); + var sampler = new MaxSamplesPerPeriod(clock, 1000L, 2L); + assertTrue(sampler.shouldSample()); + assertTrue(sampler.shouldSample()); + assertFalse(sampler.shouldSample()); + } + + @Test + public void sample_in_new_period_returns_true() { + var clock = MockUtils.mockedClockReturning(1000L, 1900L, 2000L, 2900L); + var sampler = new MaxSamplesPerPeriod(clock, 1000L, 1L); + assertTrue(sampler.shouldSample()); + assertFalse(sampler.shouldSample()); + assertTrue(sampler.shouldSample()); + assertFalse(sampler.shouldSample()); + } + +} diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/MockUtils.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/MockUtils.java new file mode 100644 index 00000000000..764e1fb6b49 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/MockUtils.java @@ -0,0 +1,24 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import java.util.Random; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class MockUtils { + + public static MonotonicNanoClock mockedClockReturning(Long ts, Long... additionalTimestamps) { + var clock = mock(MonotonicNanoClock.class); + when(clock.nanoTimeNow()).thenReturn(ts, additionalTimestamps); + return clock; + } + + // Extremely high quality randomness :D + public static Random mockedRandomReturning(Double v, Double... additionalValues) { + var rng = mock(Random.class); + when(rng.nextDouble()).thenReturn(v, additionalValues); + return rng; + } + +} diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/ProbabilisticSampleRateTest.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/ProbabilisticSampleRateTest.java new file mode 100644 index 00000000000..c1772e91296 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/ProbabilisticSampleRateTest.java @@ -0,0 +1,41 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import org.junit.Test; + +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class ProbabilisticSampleRateTest { + + private static long ms2ns(long ms) { + return TimeUnit.MILLISECONDS.toNanos(ms); + } + + @Test + public void samples_are_rate_limited_per_second() { + var clock = MockUtils.mockedClockReturning(ms2ns(10_000), ms2ns(10_500), ms2ns(10_500), ms2ns(10_501)); + var rng = MockUtils.mockedRandomReturning(0.1, 0.51, 0.49, 0.01); + var sampler = new ProbabilisticSampleRate(clock, () -> rng, 1.0); + // 1st invocation, 10 seconds (technically "infinity") since last sample. P = 1.0, sampled + assertTrue(sampler.shouldSample()); + // 2nd invocation, 0.5 seconds since last sample. rng = 0.51 >= P = 0.5, not sampled + assertFalse(sampler.shouldSample()); + // 3rd invocation, 0.5 seconds since last sample. rng = 0.49 < P = 0.5, sampled + assertTrue(sampler.shouldSample()); + // 4th invocation, 0.001 seconds since last sample. rng = 0.01 >= P = 0.001, not sampled + assertFalse(sampler.shouldSample()); + } + + @Test + public void zero_desired_sample_rate_returns_false() { + var clock = MockUtils.mockedClockReturning(ms2ns(10_000)); + var rng = MockUtils.mockedRandomReturning(0.99999999); // [0, 1) + var sampler = new ProbabilisticSampleRate(clock, () -> rng, 0.0); + + assertFalse(sampler.shouldSample()); + } + +} diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingTraceExporterTest.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingTraceExporterTest.java new file mode 100644 index 00000000000..b7dde90d28f --- /dev/null +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/tracing/SamplingTraceExporterTest.java @@ -0,0 +1,28 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.streamingvisitors.tracing; + +import org.junit.Test; + +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class SamplingTraceExporterTest { + + @Test + public void sampling_decision_is_deferred_to_provided_sampler() { + var exporter = mock(TraceExporter.class); + var sampler = mock(SamplingStrategy.class); + when(sampler.shouldSample()).thenReturn(true, false); + var samplingExporter = new SamplingTraceExporter(exporter, sampler); + + samplingExporter.maybeExport(() -> new TraceDescription(null, "")); + verify(exporter, times(1)).maybeExport(any()); + + samplingExporter.maybeExport(() -> new TraceDescription(null, "")); + verify(exporter, times(1)).maybeExport(any()); // No further invocations since last + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java index 05dea1f8567..8e7a261100a 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/aws/AwsEventFetcher.java @@ -5,6 +5,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.organization.Issue; import java.util.List; public interface AwsEventFetcher { + List<CloudEvent> getEvents(String awsRegionName); Issue createIssue(CloudEvent event); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Billing.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Billing.java index f716458542c..1e76917ebd5 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Billing.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Billing.java @@ -9,4 +9,5 @@ import com.yahoo.config.provision.ApplicationId; public interface Billing { void handleBilling(ApplicationId applicationId, String customerId); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockBilling.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockBilling.java index 20b77703160..84156cd9d2a 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockBilling.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockBilling.java @@ -10,4 +10,5 @@ public class MockBilling implements Billing { @Override public void handleBilling(ApplicationId applicationId, String customerId) {} + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java index ab2a8425897..632dbaad419 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringClient.java @@ -1,16 +1,17 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.resource; +import java.util.Collection; import java.util.List; /** - * Consumes and retrieves snapshots of resourses allocated per application. + * Consumes and retrieves snapshots of resources allocated per application. * * @author olaa */ public interface MeteringClient { - void consume(List<ResourceSnapshot> resources); + void consume(Collection<ResourceSnapshot> resources); MeteringInfo getResourceSnapshots(String tenantName, String applicationName); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringInfo.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringInfo.java index 8709315a83c..d6cb8f7fe76 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringInfo.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/MeteringInfo.java @@ -38,4 +38,5 @@ public class MeteringInfo { public Map<ApplicationId, List<ResourceSnapshot>> getSnapshotHistory() { return snapshotHistory; } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java index ed9c95bb795..10e1eb39c8a 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMeteringClient.java @@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapsh import com.yahoo.vespa.hosted.controller.api.integration.resource.MeteringClient; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -16,11 +17,11 @@ import java.util.Optional; */ public class MockMeteringClient implements MeteringClient { - private List<ResourceSnapshot> resources = new ArrayList<>(); + private Collection<ResourceSnapshot> resources = new ArrayList<>(); private Optional<MeteringInfo> meteringInfo; @Override - public void consume(List<ResourceSnapshot> resources){ + public void consume(Collection<ResourceSnapshot> resources){ this.resources = resources; } @@ -32,7 +33,7 @@ public class MockMeteringClient implements MeteringClient { }); } - public List<ResourceSnapshot> consumedResources() { + public Collection<ResourceSnapshot> consumedResources() { return this.resources; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java index 12bee2a7954..9a8adadd56e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/AwsEventReporterMaintainer.java @@ -16,10 +16,12 @@ import java.util.logging.Logger; import java.util.stream.Collectors; /** + * Automatically fetches scheduled events from AWS and submits issues detailing them. + * * @author mgimle - * Automatically fetches scheduled events from AWS and submits issues detailing them to Jira. */ public class AwsEventReporterMaintainer extends Maintainer { + private static final Logger log = Logger.getLogger(AwsEventReporterMaintainer.class.getName()); private final IssueHandler issueHandler; @@ -53,4 +55,5 @@ public class AwsEventReporterMaintainer extends Maintainer { } } } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingMaintainer.java index c6956293adf..466a79b99dc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BillingMaintainer.java @@ -29,11 +29,12 @@ public class BillingMaintainer extends Maintainer { .map(tenant -> (CloudTenant) tenant) .forEach(cloudTenant -> controller().applications().asList(cloudTenant.name()) .stream() - .forEach( application -> { + .forEach(application -> { billing.handleBilling(application.id(), cloudTenant.billingInfo().customerId()); }) ); } + } 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 2fdee305f3d..a3221ed7366 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 @@ -77,7 +77,7 @@ public class ControllerMaintenance extends AbstractComponent { contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl); nameServiceDispatcher = new NameServiceDispatcher(controller, Duration.ofSeconds(10), jobControl); costReportMaintainer = new CostReportMaintainer(controller, Duration.ofHours(2), jobControl, controller.serviceRegistry().costReportConsumer()); - resourceMeterMaintainer = new ResourceMeterMaintainer(controller, Duration.ofMinutes(30), jobControl, metric, controller.serviceRegistry().meteringService()); + resourceMeterMaintainer = new ResourceMeterMaintainer(controller, Duration.ofMinutes(1), jobControl, metric, controller.serviceRegistry().meteringService()); billingMaintainer = new BillingMaintainer(controller, Duration.ofDays(3), jobControl, billing); awsEventReporterMaintainer = new AwsEventReporterMaintainer(controller, Duration.ofDays(1), jobControl, controller.serviceRegistry().issueHandler(), awsEventFetcher); rotationStatusUpdater = new RotationStatusUpdater(controller, maintenanceInterval, jobControl); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java index 8dba0a3e813..c700ddac51c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java @@ -12,7 +12,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapsh import java.time.Clock; import java.time.Duration; -import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -47,34 +47,33 @@ public class ResourceMeterMaintainer extends Maintainer { @Override protected void maintain() { - List<Node> nodes = getNodes(); - List<ResourceSnapshot> resourceSnapshots = getResourceSnapshots(nodes); - + Collection<ResourceSnapshot> resourceSnapshots = getResourceSnapshots(allocatedNodes()); meteringClient.consume(resourceSnapshots); metric.set(METERING_LAST_REPORTED, clock.millis() / 1000, metric.createContext(Collections.emptyMap())); - metric.set(METERING_TOTAL_REPORTED, resourceSnapshots.stream() - .mapToDouble(r -> r.getCpuCores() + r.getMemoryGb() + r.getDiskGb()) // total metered resource usage, for alerting on drastic changes - .sum() - , metric.createContext(Collections.emptyMap())); + // total metered resource usage, for alerting on drastic changes + metric.set(METERING_TOTAL_REPORTED, + resourceSnapshots.stream().mapToDouble(r -> r.getCpuCores() + r.getMemoryGb() + r.getDiskGb()).sum(), + metric.createContext(Collections.emptyMap())); } - private List<Node> getNodes() { + private List<Node> allocatedNodes() { return controller().zoneRegistry().zones() .ofCloud(CloudName.from("aws")) .reachable().zones().stream() .flatMap(zone -> nodeRepository.list(zone.getId()).stream()) - .filter(node -> node.owner().isPresent() && !node.owner().get().tenant().value().equals("hosted-vespa")) - .filter(node -> node.state() == Node.State.active) + .filter(node -> node.owner().isPresent()) + .filter(node -> ! node.owner().get().tenant().value().equals("hosted-vespa")) .collect(Collectors.toList()); } - private List<ResourceSnapshot> getResourceSnapshots(List<Node> nodes) { - return new ArrayList<>(nodes.stream() - .filter(node -> node.owner().isPresent()) - .collect(Collectors.groupingBy(node -> node.owner().get(), - Collectors.collectingAndThen(Collectors.toList(), nodeList -> ResourceSnapshot.from(nodeList, clock.instant())) - )).values()); + private Collection<ResourceSnapshot> getResourceSnapshots(List<Node> nodes) { + return nodes.stream() + .collect(Collectors.groupingBy(node -> node.owner().get(), + Collectors.collectingAndThen(Collectors.toList(), + nodeList -> ResourceSnapshot.from(nodeList, + clock.instant())) + )).values(); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java index d8d7995f382..f28ce83e643 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import org.junit.Test; import java.time.Duration; +import java.util.Collection; import java.util.List; import static org.junit.Assert.assertEquals; @@ -36,7 +37,7 @@ public class ResourceMeterMaintainerTest { ResourceMeterMaintainer resourceMeterMaintainer = new ResourceMeterMaintainer(tester.controller(), Duration.ofMinutes(5), new JobControl(tester.curator()), metrics, snapshotConsumer); resourceMeterMaintainer.maintain(); - List<ResourceSnapshot> consumedResources = snapshotConsumer.consumedResources(); + Collection<ResourceSnapshot> consumedResources = snapshotConsumer.consumedResources(); // The mocked repository contains two applications, so we should also consume two ResourceSnapshots assertEquals(2, consumedResources.size()); diff --git a/document/src/main/java/net/jpountz/xxhash/package-info.java b/document/src/main/java/net/jpountz/xxhash/package-info.java new file mode 100644 index 00000000000..7599a76f46c --- /dev/null +++ b/document/src/main/java/net/jpountz/xxhash/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage(version = @Version(major = 1, minor = 3, micro = 0)) +package net.jpountz.xxhash; +import com.yahoo.osgi.annotation.ExportPackage; +import com.yahoo.osgi.annotation.Version; diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index fefed6cc05e..92a18a3094b 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -59,8 +59,9 @@ public class Flags { HOSTNAME, NODE_TYPE); public static final UnboundLongFlag THIN_POOL_GB = defineLongFlag( - "thin-pool-gb", 100, - "The size of the disk reserved for the thin pool with dynamic provisioning in AWS, in base-2 GB.", + "thin-pool-gb", -1, + "The size of the disk reserved for the thin pool with dynamic provisioning in AWS, in base-2 GB. " + + "If <0, the default is used (which may depend on the zone and node type).", "Takes effect immediately (but used only during provisioning).", NODE_TYPE); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/CoredumpMetricGatherer.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/CoredumpMetricGatherer.java index 91e7fbbc0b5..09f0bbe16a6 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/CoredumpMetricGatherer.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/CoredumpMetricGatherer.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.defaults.Defaults; import java.io.IOException; import java.io.UncheckedIOException; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.time.Instant; import java.util.Set; @@ -38,6 +39,8 @@ public class CoredumpMetricGatherer { return (int) fileWrapper.walkTree(COREDUMP_PATH) .filter(fileWrapper::isRegularFile) .count(); + } catch (NoSuchFileException e) { + return 0; } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java index 95964ec8e7f..80a280b6a8f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java @@ -46,24 +46,28 @@ public class CoredumpHandler { private final CoredumpReporter coredumpReporter; private final Path crashPatchInContainer; private final Path doneCoredumpsPath; + private final String operatorGroupName; private final Supplier<String> coredumpIdSupplier; /** * @param crashPathInContainer path inside the container where core dump are dumped * @param doneCoredumpsPath path on host where processed core dumps are stored + * @param operatorGroupName name of the group that will be set as the owner of the processed coredump */ public CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter, - Path crashPathInContainer, Path doneCoredumpsPath) { - this(terminal, coreCollector, coredumpReporter, crashPathInContainer, doneCoredumpsPath, () -> UUID.randomUUID().toString()); + Path crashPathInContainer, Path doneCoredumpsPath, String operatorGroupName) { + this(terminal, coreCollector, coredumpReporter, crashPathInContainer, doneCoredumpsPath, + operatorGroupName, () -> UUID.randomUUID().toString()); } CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter, - Path crashPathInContainer, Path doneCoredumpsPath, Supplier<String> coredumpIdSupplier) { + Path crashPathInContainer, Path doneCoredumpsPath, String operatorGroupName, Supplier<String> coredumpIdSupplier) { this.terminal = terminal; this.coreCollector = coreCollector; this.coredumpReporter = coredumpReporter; this.crashPatchInContainer = crashPathInContainer; this.doneCoredumpsPath = doneCoredumpsPath; + this.operatorGroupName = operatorGroupName; this.coredumpIdSupplier = coredumpIdSupplier; } @@ -157,7 +161,7 @@ public class CoredumpHandler { .add(LZ4_PATH, "-f", coreFile.toString(), compressedCoreFile.toString()) .setTimeout(Duration.ofMinutes(30)) .execute(); - new UnixPath(compressedCoreFile).setPermissions("rw-r-----"); + new UnixPath(compressedCoreFile).setGroup(operatorGroupName).setPermissions("rw-r-----"); Files.delete(coreFile); Path newCoredumpDirectory = doneCoredumpsPath.resolve(coredumpDirectory.getFileName()); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java index 6d3a4dbb553..1d6ccff4212 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java @@ -55,7 +55,7 @@ public class CoredumpHandlerTest { @SuppressWarnings("unchecked") private final Supplier<String> coredumpIdSupplier = mock(Supplier.class); private final CoredumpHandler coredumpHandler = new CoredumpHandler(terminal, coreCollector, coredumpReporter, - crashPathInContainer, doneCoredumpsPath, coredumpIdSupplier); + crashPathInContainer, doneCoredumpsPath, "users", coredumpIdSupplier); @Test diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index 48f846d5e7f..03cd3dd4019 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -11,6 +11,7 @@ import java.util.function.Function; import java.util.stream.Collectors; public class CapacityChecker { + private List<Node> hosts; Map<String, Node> nodeMap; private Map<Node, List<Node>> nodeChildren; @@ -523,4 +524,5 @@ public class CapacityChecker { return out.toString(); } } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java index 3c47e418b94..6a6fbf3aed0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java @@ -21,6 +21,7 @@ import java.util.*; * @author mgimle */ public class CapacityReportMaintainer extends Maintainer { + private final Metric metric; private final NodeRepository nodeRepository; private static final Logger log = Logger.getLogger(CapacityReportMaintainer.class.getName()); @@ -35,20 +36,21 @@ public class CapacityReportMaintainer extends Maintainer { @Override protected void maintain() { - if (!nodeRepository.zone().cloud().value().equals("aws")) { - CapacityChecker capacityChecker = new CapacityChecker(this.nodeRepository); - List<Node> overcommittedHosts = capacityChecker.findOvercommittedHosts(); - if (overcommittedHosts.size() != 0) { - log.log(LogLevel.WARNING, String.format("%d nodes are overcommitted! [ %s ]", overcommittedHosts.size(), - overcommittedHosts.stream().map(Node::hostname).collect(Collectors.joining(", ")))); - } - metric.set("overcommittedHosts", overcommittedHosts.size(), null); - - Optional<CapacityChecker.HostFailurePath> failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); - if (failurePath.isPresent()) { - int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); - metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); - } + if (nodeRepository.zone().cloud().value().equals("aws")) return; // Hosts and nodes are 1-1 + + CapacityChecker capacityChecker = new CapacityChecker(this.nodeRepository); + List<Node> overcommittedHosts = capacityChecker.findOvercommittedHosts(); + if (overcommittedHosts.size() != 0) { + log.log(LogLevel.WARNING, String.format("%d nodes are overcommitted! [ %s ]", overcommittedHosts.size(), + overcommittedHosts.stream().map(Node::hostname).collect(Collectors.joining(", ")))); + } + metric.set("overcommittedHosts", overcommittedHosts.size(), null); + + Optional<CapacityChecker.HostFailurePath> failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); + if (failurePath.isPresent()) { + int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); + metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); } } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index 5468ab46b63..0726ff61fdd 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -242,15 +242,10 @@ public class NodeSerializer { return flavor.with(FlavorOverrides.ofDisk(resources.field(diskKey).asDouble())); } else { - // TODO: Simplify Sept. 2019 - double bandwidth = Optional.of(resources.field(bandwidthKey)) - .filter(Inspector::valid) - .map(Inspector::asDouble) - .orElse(0.3); return new Flavor(new NodeResources(resources.field(vcpuKey).asDouble(), resources.field(memoryKey).asDouble(), resources.field(diskKey).asDouble(), - bandwidth, + resources.field(bandwidthKey).asDouble(), diskSpeedFromSlime(resources.field(diskSpeedKey)))); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java index d44b8ca2989..11d3fc23e0f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java @@ -115,7 +115,7 @@ public class NodeFailTester { List<Node> hosts = tester.createHostNodes(numberOfHosts); for (int i = 0; i < hosts.size(); i++) { tester.createReadyNodes(nodesPerHost, i * nodesPerHost, Optional.of("parent" + i), - nodeFlavors.getFlavorOrThrow("d-1-1-1"), NodeType.tenant); + new NodeResources(1, 1, 1, 0.3), NodeType.tenant); } // Create applications @@ -232,8 +232,8 @@ public class NodeFailTester { return createReadyNodes(count, startIndex, Optional.empty(), new Flavor(resources), NodeType.tenant); } - public List<Node> createReadyNodes(int count, int startIndex, Flavor flavor) { - return createReadyNodes(count, startIndex, Optional.empty(), flavor, NodeType.tenant); + private List<Node> createReadyNodes(int count, int startIndex, Optional<String> parentHostname, NodeResources resources, NodeType nodeType) { + return createReadyNodes(count, startIndex, parentHostname, new Flavor(resources), nodeType); } private List<Node> createReadyNodes(int count, int startIndex, Optional<String> parentHostname, Flavor flavor, NodeType nodeType) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java index 762fd87c2d1..50fb8290bd5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java @@ -37,16 +37,17 @@ public class AclProvisioningTest { List<Node> configServers = tester.makeConfigServers(3, "d-1-1-1", Version.fromString("6.123.456")); // Populate repo - tester.makeReadyNodes(10, "d-1-1-1"); - List<Node> dockerHost = tester.makeReadyNodes(1, "d-1-1-1", NodeType.host); + tester.makeReadyNodes(10, new NodeResources(1, 1, 1, 1)); + List<Node> dockerHost = tester.makeReadyNodes(1, new NodeResources(1, 1, 1, 1), NodeType.host); ApplicationId zoneApplication = tester.makeApplicationId(); deploy(zoneApplication, Capacity.fromRequiredNodeType(NodeType.host)); - tester.makeReadyVirtualDockerNodes(1, NodeResources.fromLegacyName("d-1-1-1"), dockerHost.get(0).hostname()); - List<Node> proxyNodes = tester.makeReadyNodes(3, "d-1-1-1", NodeType.proxy); + tester.makeReadyVirtualDockerNodes(1,new NodeResources(1, 1, 1, 1), + dockerHost.get(0).hostname()); + List<Node> proxyNodes = tester.makeReadyNodes(3, new NodeResources(1, 1, 1, 1), NodeType.proxy); // Allocate 2 nodes ApplicationId application = tester.makeApplicationId(); - List<Node> activeNodes = deploy(application, Capacity.fromCount(2, NodeResources.fromLegacyName("d-1-1-1"), false, true)); + List<Node> activeNodes = deploy(application, Capacity.fromCount(2, new NodeResources(1, 1, 1, 1), false, true)); assertEquals(2, activeNodes.size()); // Get trusted nodes for the first active node diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java index efff5333fae..1e25e861cbe 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java @@ -358,6 +358,7 @@ public class DynamicDockerAllocationTest { NodeResources.DiskSpeed.slow, hosts.get(0).flavor().get().resources().diskSpeed()); } + @SuppressWarnings("deprecation") @Test public void testSwitchingFromLegacyFlavorSyntaxToResourcesDoesNotCauseReallocation() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).flavorsConfig(flavorsConfig()).build(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 97acdc5bfaf..c1df79ce492 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -54,8 +54,8 @@ public class LoadBalancerProvisionerTest { // Provision a load balancer for each application var nodes = prepare(app1, - clusterRequest(ClusterSpec.Type.container, containerCluster1), - clusterRequest(ClusterSpec.Type.content, contentCluster)); + clusterRequest(ClusterSpec.Type.container, containerCluster1), + clusterRequest(ClusterSpec.Type.content, contentCluster)); assertEquals(1, lbApp1.get().size()); assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbApp1.get().get(0).instance().reals().size()); tester.activate(app1, nodes); @@ -212,7 +212,7 @@ public class LoadBalancerProvisionerTest { if (dynamicDockerNodes) { makeDynamicDockerNodes(specs.length * 2, capacity.type()); } else { - tester.makeReadyNodes(specs.length * 2, "d-1-1-1", capacity.type()); + tester.makeReadyNodes(specs.length * 2, new NodeResources(1, 1, 1, 0.3), capacity.type()); } Set<HostSpec> allNodes = new LinkedHashSet<>(); for (ClusterSpec spec : specs) { @@ -225,7 +225,8 @@ public class LoadBalancerProvisionerTest { List<Node> nodes = new ArrayList<>(n); for (int i = 1; i <= n; i++) { var node = Node.createDockerNode(Set.of(), "node" + i, "parent" + i, - NodeResources.fromLegacyName("d-1-1-1"), nodeType); + new NodeResources(1, 1, 1, 0.3), + nodeType); nodes.add(node); } nodes = tester.nodeRepository().database().addNodesInState(nodes, Node.State.reserved); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java index a10f0bc5f23..1836589c0ab 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java @@ -33,31 +33,34 @@ import static org.junit.Assert.assertTrue; */ public class MultigroupProvisioningTest { + private static final NodeResources small = new NodeResources(1, 1, 1, 1); + private static final NodeResources large = new NodeResources(3, 3, 3, 3); + @Test public void test_provisioning_of_multiple_groups() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); ApplicationId application1 = tester.makeApplicationId(); - tester.makeReadyNodes(21, "d-1-3-9"); - - deploy(application1, 6, 1, "d-1-3-9", tester); - deploy(application1, 6, 2, "d-1-3-9", tester); - deploy(application1, 6, 3, "d-1-3-9", tester); - deploy(application1, 6, 6, "d-1-3-9", tester); - deploy(application1, 6, 1, "d-1-3-9", tester); - deploy(application1, 6, 6, "d-1-3-9", tester); - deploy(application1, 6, 6, "d-1-3-9", tester); - deploy(application1, 6, 2, "d-1-3-9", tester); - deploy(application1, 8, 2, "d-1-3-9", tester); - deploy(application1, 9, 3, "d-1-3-9", tester); - deploy(application1, 9, 3, "d-1-3-9", tester); - deploy(application1, 9, 3, "d-1-3-9", tester); - deploy(application1,12, 4, "d-1-3-9", tester); - deploy(application1, 8, 4, "d-1-3-9", tester); - deploy(application1,12, 4, "d-1-3-9", tester); - deploy(application1, 8, 2, "d-1-3-9", tester); - deploy(application1, 6, 3, "d-1-3-9", tester); + tester.makeReadyNodes(21, small); + + deploy(application1, 6, 1, small, tester); + deploy(application1, 6, 2, small, tester); + deploy(application1, 6, 3, small, tester); + deploy(application1, 6, 6, small, tester); + deploy(application1, 6, 1, small, tester); + deploy(application1, 6, 6, small, tester); + deploy(application1, 6, 6, small, tester); + deploy(application1, 6, 2, small, tester); + deploy(application1, 8, 2, small, tester); + deploy(application1, 9, 3, small, tester); + deploy(application1, 9, 3, small, tester); + deploy(application1, 9, 3, small, tester); + deploy(application1,12, 4, small, tester); + deploy(application1, 8, 4, small, tester); + deploy(application1,12, 4, small, tester); + deploy(application1, 8, 2, small, tester); + deploy(application1, 6, 3, small, tester); } /** @@ -65,13 +68,14 @@ public class MultigroupProvisioningTest { * due to asymmetric group sizes after step 2 (second group has 3 additional retired nodes). * We probably need to switch to a multipass group allocation procedure to fix this case. */ - @Test @Ignore + @Ignore + @Test public void test_provisioning_of_groups_with_asymmetry() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); ApplicationId application1 = tester.makeApplicationId(); - tester.makeReadyNodes(21, "d-1-3-9"); + tester.makeReadyNodes(21, large); deploy(application1, 12, 2, tester); deploy(application1, 9, 3, tester); @@ -84,12 +88,12 @@ public class MultigroupProvisioningTest { ApplicationId application1 = tester.makeApplicationId(); - tester.makeReadyNodes(10, "d-1-1-1"); - tester.makeReadyNodes(10, "d-3-3-3"); + tester.makeReadyNodes(10, small); + tester.makeReadyNodes(10, large); - deploy(application1, 8, 1, "d-1-1-1", tester); - deploy(application1, 8, 1, "d-3-3-3", tester); - deploy(application1, 8, 8, "d-3-3-3", tester); + deploy(application1, 8, 1, small, tester); + deploy(application1, 8, 1, large, tester); + deploy(application1, 8, 8, large, tester); } @Test @@ -98,10 +102,10 @@ public class MultigroupProvisioningTest { ApplicationId application1 = tester.makeApplicationId(); - tester.makeReadyNodes(10, "d-1-1-1"); + tester.makeReadyNodes(10, small); - deploy(application1, Capacity.fromNodeCount(1, Optional.of("d-1-1-1"), true, true), 1, tester); - deploy(application1, Capacity.fromNodeCount(2, Optional.of("d-1-1-1"), true, true), 2, tester); + deploy(application1, Capacity.fromCount(1, Optional.of(small), true, true), 1, tester); + deploy(application1, Capacity.fromCount(2, Optional.of(small), true, true), 2, tester); } @Test @@ -110,11 +114,11 @@ public class MultigroupProvisioningTest { ApplicationId application1 = tester.makeApplicationId(); - tester.makeReadyNodes(10, "d-1-1-1"); - tester.makeReadyNodes(10, "d-3-3-3"); + tester.makeReadyNodes(10, small); + tester.makeReadyNodes(10, large); - deploy(application1, Capacity.fromNodeCount(1, Optional.of("d-1-1-1"), true, true), 1, tester); - deploy(application1, Capacity.fromNodeCount(2, Optional.of("d-3-3-3"), true, true), 2, tester); + deploy(application1, Capacity.fromCount(1, Optional.of(small), true, true), 1, tester); + deploy(application1, Capacity.fromCount(2, Optional.of(large), true, true), 2, tester); } @Test @@ -123,11 +127,11 @@ public class MultigroupProvisioningTest { ApplicationId application1 = tester.makeApplicationId(); - tester.makeReadyNodes(10, "d-1-1-1"); - tester.makeReadyNodes(10, "d-3-3-3"); + tester.makeReadyNodes(10, small); + tester.makeReadyNodes(10, large); - deploy(application1, 8, 1, "d-1-1-1", tester); - deploy(application1, 8, 1, "d-3-3-3", tester); + deploy(application1, 8, 1, small, tester); + deploy(application1, 8, 1, large, tester); // Expire small nodes tester.advanceTime(Duration.ofDays(7)); @@ -136,19 +140,19 @@ public class MultigroupProvisioningTest { tester.clock(), Collections.singletonMap(application1, new MockDeployer.ApplicationContext(application1, cluster(), - Capacity.fromNodeCount(8, Optional.of("d-3-3-3"), false, true), 1))); + Capacity.fromCount(8, Optional.of(large), false, true), 1))); new RetiredExpirer(tester.nodeRepository(), tester.orchestrator(), deployer, tester.clock(), Duration.ofDays(30), Duration.ofHours(12)).run(); - assertEquals(8, tester.getNodes(application1, Node.State.inactive).resources(new NodeResources(1, 1, 1, 0.3)).size()); - deploy(application1, 8, 8, "d-3-3-3", tester); + assertEquals(8, tester.getNodes(application1, Node.State.inactive).resources(small).size()); + deploy(application1, 8, 8, large, tester); } - private void deploy(ApplicationId application, int nodeCount, int groupCount, String flavor, ProvisioningTester tester) { - deploy(application, Capacity.fromNodeCount(nodeCount, Optional.of(flavor), false, true), groupCount, tester); + private void deploy(ApplicationId application, int nodeCount, int groupCount, NodeResources resources, ProvisioningTester tester) { + deploy(application, Capacity.fromCount(nodeCount, Optional.of(resources), false, true), groupCount, tester); } private void deploy(ApplicationId application, int nodeCount, int groupCount, ProvisioningTester tester) { - deploy(application, Capacity.fromNodeCount(nodeCount, Optional.of("d-3-3-3"), false, true), groupCount, tester); + deploy(application, Capacity.fromCount(nodeCount, Optional.of(large), false, true), groupCount, tester); } private void deploy(ApplicationId application, Capacity capacity, int wantedGroups, ProvisioningTester tester) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 851a734674f..58440372821 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -438,7 +438,7 @@ public class ProvisioningTest { ClusterSpec.Id.from("music"), new com.yahoo.component.Version(4, 5, 6), false); - tester.prepare(application, cluster, Capacity.fromNodeCount(5, Optional.empty(), false, false), 1); + tester.prepare(application, cluster, Capacity.fromCount(5, Optional.empty(), false, false), 1); // No exception; Success } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 50657e7eab2..844677395f4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -238,6 +238,9 @@ public class ProvisioningTester { public List<Node> makeReadyNodes(int n, String flavor, NodeType type) { return makeReadyNodes(n, asFlavor(flavor, type), type, 0); } + public List<Node> makeReadyNodes(int n, NodeResources resources, NodeType type) { + return makeReadyNodes(n, new Flavor(resources), type, 0); + } public List<Node> makeReadyNodes(int n, NodeResources resources, NodeType type, int ipAddressPoolSize) { return makeReadyNodes(n, new Flavor(resources), type, ipAddressPoolSize); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java index 8342dfb16df..5b52f757dad 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -70,12 +70,8 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { // Includes all available commands in 3.4, except 'wchc' and 'wchp' // Mandatory when using ZooKeeper 3.5 sb.append("4lw.commands.whitelist=conf,cons,crst,dump,envi,mntr,ruok,srst,srvr,stat,wchs").append("\n"); - if (config.server().size() > 1) { - ensureThisServerIsRepresented(config.myid(), config.server()); - for (ZookeeperServerConfig.Server server : config.server()) { - addServerToCfg(sb, server); - } - } + ensureThisServerIsRepresented(config.myid(), config.server()); + config.server().forEach(server -> addServerToCfg(sb, server)); return sb.toString(); } diff --git a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java index db1852d9d2a..3f33892fd45 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java @@ -29,8 +29,8 @@ public class ZooKeeperServerTest { ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); builder.myidFile(idFile.getAbsolutePath()); - builder.server(newServer(1, "foo", 123, 321)); - builder.myid(1); + builder.server(newServer(0, "foo", 123, 321)); + builder.myid(0); createServer(builder); validateConfigFileSingleHost(cfgFile); validateIdFile(idFile, ""); @@ -42,9 +42,9 @@ public class ZooKeeperServerTest { File idFile = folder.newFile(); ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); - builder.server(newServer(1, "foo", 123, 321)); - builder.server(newServer(2, "bar", 234, 432)); - builder.server(newServer(3, "baz", 345, 543)); + builder.server(newServer(0, "foo", 123, 321)); + builder.server(newServer(1, "bar", 234, 432)); + builder.server(newServer(2, "baz", 345, 543)); builder.myidFile(idFile.getAbsolutePath()); builder.myid(1); createServer(builder); @@ -59,16 +59,16 @@ public class ZooKeeperServerTest { @Test(expected = RuntimeException.class) public void require_that_this_id_must_be_present_amongst_servers() { ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); - builder.server(newServer(2, "bar", 234, 432)); - builder.server(newServer(3, "baz", 345, 543)); - builder.myid(1); + builder.server(newServer(1, "bar", 234, 432)); + builder.server(newServer(2, "baz", 345, 543)); + builder.myid(0); createServer(builder); } @Test public void juteMaxBufferCanBeSet() throws IOException { ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); - builder.myid(1); + builder.myid(0); File idFile = folder.newFile(); File cfgFile = folder.newFile(); @@ -110,7 +110,8 @@ public class ZooKeeperServerTest { "clientPort=2181\n" + "autopurge.purgeInterval=1\n" + "autopurge.snapRetainCount=15\n" + - "4lw.commands.whitelist=conf,cons,crst,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n"; + "4lw.commands.whitelist=conf,cons,crst,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n" + + "server.0=foo:321:123\n"; validateConfigFile(cfgFile, expected); } @@ -126,9 +127,9 @@ public class ZooKeeperServerTest { "autopurge.purgeInterval=1\n" + "autopurge.snapRetainCount=15\n" + "4lw.commands.whitelist=conf,cons,crst,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n" + - "server.1=foo:321:123\n" + - "server.2=bar:432:234\n" + - "server.3=baz:543:345\n"; + "server.0=foo:321:123\n" + + "server.1=bar:432:234\n" + + "server.2=baz:543:345\n"; validateConfigFile(cfgFile, expected); } |