diff options
51 files changed, 998 insertions, 606 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java index 09990c7b9de..11736256d1b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java @@ -2,9 +2,12 @@ package com.yahoo.vespa.model.container; import ai.vespa.models.evaluation.ModelsEvaluator; +import ai.vespa.models.handler.ModelsEvaluationHandler; +import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.searchdefinition.derived.RankProfileList; import com.yahoo.vespa.config.search.RankProfilesConfig; import com.yahoo.vespa.config.search.core.RankingConstantsConfig; +import com.yahoo.vespa.model.container.component.Handler; import java.util.List; import java.util.Objects; @@ -16,12 +19,17 @@ import java.util.Objects; */ public class ContainerModelEvaluation implements RankProfilesConfig.Producer, RankingConstantsConfig.Producer { + private final static String EVALUATOR_NAME = ModelsEvaluator.class.getName(); + private final static String REST_HANDLER_NAME = ModelsEvaluationHandler.class.getName(); + private final static String BUNDLE_NAME = "model-evaluation"; + /** Global rank profiles, aka models */ private final RankProfileList rankProfileList; public ContainerModelEvaluation(ContainerCluster cluster, RankProfileList rankProfileList) { this.rankProfileList = Objects.requireNonNull(rankProfileList, "rankProfileList cannot be null"); - cluster.addSimpleComponent(ModelsEvaluator.class.getName(), null, "model-evaluation"); + cluster.addSimpleComponent(EVALUATOR_NAME, null, BUNDLE_NAME); + cluster.addComponent(ContainerModelEvaluation.getHandler()); } public void prepare(List<Container> containers) { @@ -38,4 +46,14 @@ public class ContainerModelEvaluation implements RankProfilesConfig.Producer, Ra rankProfileList.getConfig(builder); } + public static Handler<?> getHandler() { + Handler<?> handler = new Handler<>(new ComponentModel(REST_HANDLER_NAME, null, BUNDLE_NAME)); + String binding = ModelsEvaluationHandler.API_ROOT + "/" + ModelsEvaluationHandler.VERSION_V1; + handler.addServerBindings("http://*/" + binding, + "https://*/" + binding, + "http://*/" + binding + "/*", + "https://*/" + binding + "/*"); + return handler; + } + } diff --git a/config-model/src/main/perl/vespa-deploy b/config-model/src/main/perl/vespa-deploy index 8d2d65b5551..4ed8311d7ae 100755 --- a/config-model/src/main/perl/vespa-deploy +++ b/config-model/src/main/perl/vespa-deploy @@ -240,6 +240,14 @@ sub usage { print "Usage: vespa-deploy [-h] [-v] [-f] [-t] [-p] [-V] [<command>] [args]\n"; print "Supported commands: 'upload', 'prepare', 'activate', 'fetch' and 'help'\n"; print "Supported options: '-h' (help), '-v' (verbose), '-f' (force/ignore validation errors), '-t' (timeout in seconds), '-p' (config server http port)\n"; + print " '-h' (help)\n"; + print " '-v' (verbose)\n"; + print " '-n' (dry-run)\n"; + print " '-f' (force/ignore validation errors)\n"; + print " '-t <timeout>' (timeout in seconds)\n"; + print " '-c <server>' (config server hostname)\n"; + print " '-p <port>' (config server http port)\n\n"; + print "Try 'vespa-deploy help <command>' to get more help\n"; } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/ml/ModelEvaluationTest.java b/config-model/src/test/java/com/yahoo/vespa/model/ml/ModelEvaluationTest.java index b7b3fc99e20..9e26caf2cb4 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/ml/ModelEvaluationTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/ml/ModelEvaluationTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.model.ml; import ai.vespa.models.evaluation.Model; import ai.vespa.models.evaluation.ModelsEvaluator; import ai.vespa.models.evaluation.RankProfilesConfigImporter; +import ai.vespa.models.handler.ModelsEvaluationHandler; import com.yahoo.component.ComponentId; import com.yahoo.config.FileReference; import com.yahoo.config.application.api.ApplicationPackage; @@ -80,6 +81,10 @@ public class ModelEvaluationTest { ContainerCluster cluster = model.getContainerClusters().get("container"); assertNotNull(cluster.getComponentsMap().get(new ComponentId(ModelsEvaluator.class.getName()))); + assertNotNull(cluster.getComponentsMap().get(new ComponentId(ModelsEvaluationHandler.class.getName()))); + assertTrue(cluster.getHandlers().stream() + .anyMatch(h -> h.getComponentId().toString().equals(ModelsEvaluationHandler.class.getName()))); + RankProfilesConfig.Builder b = new RankProfilesConfig.Builder(); cluster.getConfig(b); RankProfilesConfig config = new RankProfilesConfig(b); diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index 55d7de90f33..7f6e7f08e3d 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -148,7 +148,6 @@ public class HandlersConfigurerDi { protected void configure() { bind(com.yahoo.container.Container.class).toInstance(vespaContainer); bind(com.yahoo.statistics.Statistics.class).toInstance(Statistics.nullImplementation); - bind(Linguistics.class).toInstance(new SimpleLinguistics()); bind(com.yahoo.container.protect.FreezeDetector.class).toInstance( new com.yahoo.container.protect.FreezeDetector( new DiagnosticsConfig(new DiagnosticsConfig.Builder().disabled(true)))); diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java index 42780f75a6c..ad99218b860 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java @@ -1,7 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.core.config.testutil; +import com.google.inject.AbstractModule; import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Scopes; import com.yahoo.component.AbstractComponent; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.config.subscription.ConfigSourceSet; @@ -10,6 +13,8 @@ import com.yahoo.container.di.CloudSubscriberFactory; import com.yahoo.container.di.ComponentDeconstructor; import com.yahoo.container.core.config.HandlersConfigurerDi; import com.yahoo.jdisc.handler.RequestHandler; +import com.yahoo.language.Linguistics; +import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.osgi.MockOsgi; import java.io.File; @@ -92,7 +97,7 @@ public class HandlersConfigurerTestWrapper { container, configId, testDeconstructor, - Guice.createInjector(), + guiceInjector(), mockOsgi); this.container = container; } @@ -111,7 +116,7 @@ public class HandlersConfigurerTestWrapper { public void reloadConfig() { configurer.reloadConfig(++lastGeneration); - configurer.getNewComponentGraph(Guice.createInjector(), false); + configurer.getNewComponentGraph(guiceInjector(), false); } public void shutdown() { @@ -125,4 +130,14 @@ public class HandlersConfigurerTestWrapper { return container.getRequestHandlerRegistry(); } + private static Injector guiceInjector() { + return Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + // Needed by e.g. SearchHandler + bind(Linguistics.class).to(SimpleLinguistics.class).in(Scopes.SINGLETON); + } + }); + } + } diff --git a/container-search/src/main/java/com/yahoo/fs4/BasicPacket.java b/container-search/src/main/java/com/yahoo/fs4/BasicPacket.java index 85e1aef3da0..6f87e45af25 100644 --- a/container-search/src/main/java/com/yahoo/fs4/BasicPacket.java +++ b/container-search/src/main/java/com/yahoo/fs4/BasicPacket.java @@ -4,9 +4,11 @@ package com.yahoo.fs4; import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; import com.yahoo.log.LogLevel; +import com.yahoo.prelude.fastsearch.TimeoutException; import net.jpountz.lz4.LZ4Compressor; import net.jpountz.lz4.LZ4Factory; +import java.io.IOException; import java.nio.ByteBuffer; import java.util.Optional; import java.util.logging.Logger; @@ -325,4 +327,20 @@ public abstract class BasicPacket { return false; } + /** + * Throws an IOException if the packet is not of the expected type + */ + public void ensureInstanceOf(Class<? extends BasicPacket> type, String name) throws IOException { + if ((type.isAssignableFrom(getClass()))) return; + + if (this instanceof ErrorPacket) { + ErrorPacket errorPacket = (ErrorPacket) this; + if (errorPacket.getErrorCode() == 8) + throw new TimeoutException("Query timed out in " + name); + else + throw new IOException("Received error from backend in " + name + ": " + this); + } else { + throw new IOException("Received " + this + " when expecting " + type); + } + } } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4CloseableChannel.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4CloseableChannel.java new file mode 100644 index 00000000000..dc95f83365e --- /dev/null +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4CloseableChannel.java @@ -0,0 +1,351 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.prelude.fastsearch; + +import com.yahoo.fs4.BasicPacket; +import com.yahoo.fs4.ChannelTimeoutException; +import com.yahoo.fs4.DocsumPacket; +import com.yahoo.fs4.GetDocSumsPacket; +import com.yahoo.fs4.Packet; +import com.yahoo.fs4.QueryPacket; +import com.yahoo.fs4.QueryResultPacket; +import com.yahoo.fs4.mplex.Backend; +import com.yahoo.fs4.mplex.FS4Channel; +import com.yahoo.fs4.mplex.InvalidChannelException; +import com.yahoo.prelude.fastsearch.VespaBackEndSearcher.FillHitsResult; +import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.dispatch.CloseableChannel; +import com.yahoo.search.result.ErrorMessage; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; + +import java.io.IOException; +import java.util.Iterator; +import java.util.Optional; +import java.util.logging.Level; +import java.util.logging.Logger; + +import static com.yahoo.prelude.fastsearch.VespaBackEndSearcher.hitIterator; + +/** + * {@link CloseableChannel} implementation for FS4 nodes and fdispatch + * + * @author ollivir + */ +public class FS4CloseableChannel extends CloseableChannel { + private final VespaBackEndSearcher searcher; + private FS4Channel channel; + private final Optional<Integer> distributionKey; + + public FS4CloseableChannel(VespaBackEndSearcher searcher, Query query, FS4ResourcePool fs4ResourcePool, String hostname, int port, + int distributionKey) { + this.searcher = searcher; + this.distributionKey = Optional.of(distributionKey); + + Backend backend = fs4ResourcePool.getBackend(hostname, port); + this.channel = backend.openChannel(); + channel.setQuery(query); + } + + // fdispatch code path + public FS4CloseableChannel(VespaBackEndSearcher searcher, Query query, Backend backend) { + this.searcher = searcher; + this.distributionKey = Optional.empty(); + this.channel = backend.openChannel(); + channel.setQuery(query); + } + + @Override + public Result search(Query query, QueryPacket queryPacket, CacheKey cacheKey) throws IOException { + if (isLoggingFine()) + getLogger().finest("sending query packet"); + + try { + boolean couldSend = channel.sendPacket(queryPacket); + if (!couldSend) + return new Result(query, ErrorMessage.createBackendCommunicationError("Could not reach '" + getName() + "'")); + } catch (InvalidChannelException e) { + return new Result(query, ErrorMessage.createBackendCommunicationError("Invalid channel " + getName())); + } catch (IllegalStateException e) { + return new Result(query, ErrorMessage.createBackendCommunicationError("Illegal state in FS4: " + e.getMessage())); + } + + BasicPacket[] basicPackets; + + try { + basicPackets = channel.receivePackets(query.getTimeLeft(), 1); + } catch (ChannelTimeoutException e) { + return new Result(query, ErrorMessage.createTimeout("Timeout while waiting for " + getName())); + } catch (InvalidChannelException e) { + return new Result(query, ErrorMessage.createBackendCommunicationError("Invalid channel for " + getName())); + } + + if (basicPackets.length == 0) { + return new Result(query, ErrorMessage.createBackendCommunicationError(getName() + " got no packets back")); + } + + if (isLoggingFine()) + getLogger().finest("got packets " + basicPackets.length + " packets"); + + basicPackets[0].ensureInstanceOf(QueryResultPacket.class, getName()); + QueryResultPacket resultPacket = (QueryResultPacket) basicPackets[0]; + + if (isLoggingFine()) + getLogger().finest("got query packet. " + "docsumClass=" + query.getPresentation().getSummary()); + + if (query.getPresentation().getSummary() == null) + query.getPresentation().setSummary(searcher.getDefaultDocsumClass()); + + Result result = new Result(query); + + searcher.addMetaInfo(query, queryPacket.getQueryPacketData(), resultPacket, result); + + searcher.addUnfilledHits(result, resultPacket.getDocuments(), false, queryPacket.getQueryPacketData(), cacheKey, distributionKey); + Packet[] packets; + CacheControl cacheControl = searcher.getCacheControl(); + PacketWrapper packetWrapper = cacheControl.lookup(cacheKey, query); + + if (packetWrapper != null) { + cacheControl.updateCacheEntry(cacheKey, query, resultPacket); + } else { + if (resultPacket.getCoverageFeature() && !resultPacket.getCoverageFull()) { + // Don't add error here, it was done in first phase + // No check if packetWrapper already exists, since incomplete + // first phase data won't be cached anyway. + } else { + packets = new Packet[1]; + packets[0] = resultPacket; + cacheControl.cache(cacheKey, query, new DocsumPacketKey[0], packets, distributionKey); + } + } + return result; + } + + @Override + public void partialFill(Result result, String summaryClass) { + Packet[] receivedPackets; + DocsumPacketKey[] packetKeys; + + CacheKey cacheKey = null; + PacketWrapper packetWrapper = null; + if (searcher.getCacheControl().useCache(channel.getQuery())) { + cacheKey = fetchCacheKeyFromHits(result.hits(), summaryClass); + if (cacheKey == null) { + QueryPacket queryPacket = QueryPacket.create(channel.getQuery()); + cacheKey = new CacheKey(queryPacket); + } + packetWrapper = cacheLookupTwoPhase(cacheKey, result, summaryClass); + } + + if (countFastHits(result) > 0) { + packetKeys = getPacketKeys(result, summaryClass, false); + if (packetKeys.length == 0) { + receivedPackets = new Packet[0]; + } else { + try { + receivedPackets = fetchSummaries(result, summaryClass); + } catch (InvalidChannelException e) { + result.hits() + .addError(ErrorMessage.createBackendCommunicationError("Invalid channel " + getName() + " (summary fetch)")); + return; + } catch (ChannelTimeoutException e) { + result.hits().addError(ErrorMessage.createTimeout("timeout waiting for summaries from " + getName())); + return; + } catch (IOException e) { + result.hits().addError(ErrorMessage.createBackendCommunicationError( + "IO error while talking on channel " + getName() + " (summary fetch): " + e.getMessage())); + return; + } + if (receivedPackets.length == 0) { + result.hits() + .addError(ErrorMessage.createBackendCommunicationError(getName() + " got no packets back (summary fetch)")); + return; + } + } + } else { + packetKeys = new DocsumPacketKey[0]; + receivedPackets = new Packet[0]; + } + + int skippedHits; + try { + FillHitsResult fillHitsResult = searcher.fillHits(result, receivedPackets, summaryClass); + skippedHits = fillHitsResult.skippedHits; + if (fillHitsResult.error != null) { + result.hits().addError(ErrorMessage.createTimeout(fillHitsResult.error)); + return; + } + } catch (TimeoutException e) { + result.hits().addError(ErrorMessage.createTimeout(e.getMessage())); + return; + } catch (IOException e) { + result.hits().addError(ErrorMessage.createBackendCommunicationError( + "Error filling hits with summary fields, source: " + getName() + " Exception thrown: " + e.getMessage())); + return; + } + if (skippedHits == 0 && packetWrapper != null) { + searcher.getCacheControl().updateCacheEntry(cacheKey, channel.getQuery(), packetKeys, receivedPackets); + } + + if (skippedHits > 0) + result.hits().addError( + ErrorMessage.createEmptyDocsums("Missing hit data for summary '" + summaryClass + "' for " + skippedHits + " hits")); + result.analyzeHits(); + + if (channel.getQuery().getTraceLevel() >= 3) { + int hitNumber = 0; + for (Iterator<com.yahoo.search.result.Hit> i = hitIterator(result); i.hasNext();) { + com.yahoo.search.result.Hit hit = i.next(); + if (!(hit instanceof FastHit)) + continue; + FastHit fastHit = (FastHit) hit; + + String traceMsg = "Hit: " + (hitNumber++) + " from " + (fastHit.isCached() ? "cache" : "backend"); + if (!fastHit.isFilled(summaryClass)) + traceMsg += ". Error, hit, not filled"; + channel.getQuery().trace(traceMsg, false, 3); + } + } + } + + @Override + public void closeChannel() { + if (channel != null) { + channel.close(); + channel = null; + } + } + + private PacketWrapper cacheLookupTwoPhase(CacheKey cacheKey, Result result, String summaryClass) { + Query query = result.getQuery(); + PacketWrapper packetWrapper = searcher.getCacheControl().lookup(cacheKey, query); + + if (packetWrapper == null) { + return null; + } + if (packetWrapper.getNumPackets() != 0) { + for (Iterator<Hit> i = hitIterator(result); i.hasNext();) { + Hit hit = i.next(); + + if (hit instanceof FastHit) { + FastHit fastHit = (FastHit) hit; + DocsumPacketKey key = new DocsumPacketKey(fastHit.getGlobalId(), fastHit.getPartId(), summaryClass); + + if (searcher.fillHit(fastHit, (DocsumPacket) packetWrapper.getPacket(key), summaryClass).ok) { + fastHit.setCached(true); + } + + } + } + result.hits().setSorted(false); + result.analyzeHits(); + } + + return packetWrapper; + } + + private CacheKey fetchCacheKeyFromHits(HitGroup hits, String summaryClass) { + for (Iterator<Hit> i = hits.unorderedDeepIterator(); i.hasNext();) { + Hit h = i.next(); + if (h instanceof FastHit) { + FastHit hit = (FastHit) h; + if (hit.isFilled(summaryClass)) { + continue; + } + if (hit.getCacheKey() != null) { + return hit.getCacheKey(); + } + } + } + return null; + } + + private int countFastHits(Result result) { + int count = 0; + for (Iterator<Hit> i = hitIterator(result); i.hasNext();) { + if (i.next() instanceof FastHit) + count++; + } + return count; + } + + private Packet[] fetchSummaries(Result result, String summaryClass) + throws InvalidChannelException, ChannelTimeoutException, ClassCastException, IOException { + + BasicPacket[] receivedPackets; + boolean summaryNeedsQuery = searcher.summaryNeedsQuery(result.getQuery()); + if (result.getQuery().getTraceLevel() >= 3) + result.getQuery().trace((summaryNeedsQuery ? "Resending " : "Not resending ") + "query during document summary fetching", 3); + + GetDocSumsPacket docsumsPacket = GetDocSumsPacket.create(result, summaryClass, summaryNeedsQuery); + int compressionLimit = result.getQuery().properties().getInteger(FastSearcher.PACKET_COMPRESSION_LIMIT, 0); + docsumsPacket.setCompressionLimit(compressionLimit); + if (compressionLimit != 0) { + docsumsPacket.setCompressionType(result.getQuery().properties().getString(FastSearcher.PACKET_COMPRESSION_TYPE, "lz4")); + } + + boolean couldSend = channel.sendPacket(docsumsPacket); + if (!couldSend) + throw new IOException("Could not successfully send GetDocSumsPacket."); + receivedPackets = channel.receivePackets(result.getQuery().getTimeLeft(), docsumsPacket.getNumDocsums() + 1); + + return convertBasicPackets(receivedPackets); + } + + /** + * Returns an array of the hits contained in a result + * + * @param filled + * true to return all hits, false to return only unfilled hits + * @return array of docids, empty array if no hits + */ + private DocsumPacketKey[] getPacketKeys(Result result, String summaryClass, boolean filled) { + DocsumPacketKey[] packetKeys = new DocsumPacketKey[result.getHitCount()]; + int x = 0; + + for (Iterator<com.yahoo.search.result.Hit> i = hitIterator(result); i.hasNext();) { + com.yahoo.search.result.Hit hit = i.next(); + if (hit instanceof FastHit) { + FastHit fastHit = (FastHit) hit; + if (filled || !fastHit.isFilled(summaryClass)) { + packetKeys[x] = new DocsumPacketKey(fastHit.getGlobalId(), fastHit.getPartId(), summaryClass); + x++; + } + } + } + if (x < packetKeys.length) { + DocsumPacketKey[] tmp = new DocsumPacketKey[x]; + + System.arraycopy(packetKeys, 0, tmp, 0, x); + return tmp; + } else { + return packetKeys; + } + } + + private static Packet[] convertBasicPackets(BasicPacket[] basicPackets) throws ClassCastException { + // trying to cast a BasicPacket[] to Packet[] will compile, + // but lead to a runtime error. At least that's what I got + // from testing and reading the specification. I'm just happy + // if someone tells me what's the proper Java way of doing + // this. -SK + Packet[] packets = new Packet[basicPackets.length]; + + for (int i = 0; i < basicPackets.length; i++) { + packets[i] = (Packet) basicPackets[i]; + } + return packets; + } + + private String getName() { + return searcher.getName(); + } + + private Logger getLogger() { + return searcher.getLogger(); + } + + private boolean isLoggingFine() { + return getLogger().isLoggable(Level.FINE); + } +} diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index 333d3970cc4..d34d119c1fe 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -1,18 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.fastsearch; -import java.util.Optional; - import com.yahoo.compress.CompressionType; import com.yahoo.container.search.LegacyEmulationConfig; import com.yahoo.fs4.BasicPacket; import com.yahoo.fs4.ChannelTimeoutException; -import com.yahoo.fs4.GetDocSumsPacket; -import com.yahoo.fs4.Packet; import com.yahoo.fs4.PingPacket; import com.yahoo.fs4.PongPacket; import com.yahoo.fs4.QueryPacket; -import com.yahoo.fs4.QueryResultPacket; import com.yahoo.fs4.mplex.Backend; import com.yahoo.fs4.mplex.FS4Channel; import com.yahoo.fs4.mplex.InvalidChannelException; @@ -29,13 +24,11 @@ import com.yahoo.search.grouping.GroupingRequest; import com.yahoo.search.grouping.request.GroupingOperation; import com.yahoo.search.query.Ranking; import com.yahoo.search.result.ErrorMessage; -import com.yahoo.search.result.Hit; -import com.yahoo.search.result.HitGroup; import com.yahoo.search.searchchain.Execution; import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; -import java.util.Iterator; +import java.util.Optional; import java.util.logging.Level; import static com.yahoo.container.util.Util.quote; @@ -99,15 +92,6 @@ public class FastSearcher extends VespaBackEndSearcher { this.dispatcher = dispatcher; } - private int countFastHits(Result result) { - int count = 0; - for (Iterator<Hit> i = hitIterator(result); i.hasNext();) { - if (i.next() instanceof FastHit) - count++; - } - return count; - } - /** * Pings the backend. Does not propagate to other searchers. */ @@ -153,7 +137,7 @@ public class FastSearcher extends VespaBackEndSearcher { } try { - ensureInstanceOf(PongPacket.class, packets[0], name); + packets[0].ensureInstanceOf(PongPacket.class, name); } catch (TimeoutException e) { return new Pong(ErrorMessage.createTimeout(e.getMessage())); } catch (IOException e) { @@ -176,9 +160,7 @@ public class FastSearcher extends VespaBackEndSearcher { if (dispatcher.searchCluster().groupSize() == 1) forceSinglePassGrouping(query); try(CloseableChannel channel = getChannel(query)) { - channel.setQuery(query); - - Result result = searchTwoPhase(channel, query, queryPacket, cacheKey); + Result result = channel.search(query, queryPacket, cacheKey); if (query.properties().getBoolean(Ranking.RANKFEATURES, false)) { // There is currently no correct choice for which @@ -214,7 +196,7 @@ public class FastSearcher extends VespaBackEndSearcher { } /** - * Returns an interface object to issue a search request over. + * Returns a request interface object for the given query. * Normally this is built from the backend field of this instance, which connects to the dispatch node * this component talks to (which is why this instance was chosen by the cluster controller). However, * under certain conditions we will instead return an interface which connects directly to the relevant @@ -222,24 +204,24 @@ public class FastSearcher extends VespaBackEndSearcher { */ private CloseableChannel getChannel(Query query) { if (query.properties().getBoolean(dispatchInternal, false)) { - Optional<CloseableChannel> dispatchedChannel = dispatcher.getDispatchedChannel(query); + Optional<CloseableChannel> dispatchedChannel = dispatcher.getDispatchedChannel(this, query); if (dispatchedChannel.isPresent()) { return dispatchedChannel.get(); } } if (!query.properties().getBoolean(dispatchDirect, true)) - return new CloseableChannel(dispatchBackend); + return new FS4CloseableChannel(this, query, dispatchBackend); if (query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) - return new CloseableChannel(dispatchBackend); + return new FS4CloseableChannel(this, query, dispatchBackend); Optional<SearchCluster.Node> directDispatchRecipient = dispatcher.searchCluster().directDispatchTarget(); if (!directDispatchRecipient.isPresent()) - return new CloseableChannel(dispatchBackend); + return new FS4CloseableChannel(this, query, dispatchBackend); // Dispatch directly to the single, local search node + SearchCluster.Node local = directDispatchRecipient.get(); query.trace(false, 2, "Dispatching directly to ", directDispatchRecipient.get()); - return new CloseableChannel(fs4ResourcePool.getBackend(directDispatchRecipient.get().hostname(), - directDispatchRecipient.get().fs4port()), Optional.of(directDispatchRecipient.get().key())); + return new FS4CloseableChannel(this, query, fs4ResourcePool, local.hostname(), local.fs4port(), local.key()); } /** @@ -267,86 +249,8 @@ public class FastSearcher extends VespaBackEndSearcher { return; } - CacheKey cacheKey = null; - PacketWrapper packetWrapper = null; - if (getCacheControl().useCache(query)) { - cacheKey = fetchCacheKeyFromHits(result.hits(), summaryClass); - if (cacheKey == null) { - QueryPacket queryPacket = QueryPacket.create(query); - cacheKey = new CacheKey(queryPacket); - } - packetWrapper = cacheLookupTwoPhase(cacheKey, result, summaryClass); - } - - Packet[] receivedPackets; - try(CloseableChannel channel = getChannel(query)) { - channel.setQuery(query); - DocsumPacketKey[] packetKeys; - - if (countFastHits(result) > 0) { - packetKeys = getPacketKeys(result, summaryClass, false); - if (packetKeys.length == 0) { - receivedPackets = new Packet[0]; - } else { - try { - receivedPackets = fetchSummaries(channel, result, summaryClass); - } catch (InvalidChannelException e) { - result.hits().addError(ErrorMessage.createBackendCommunicationError("Invalid channel " + getName() + " (summary fetch)")); - return; - } catch (ChannelTimeoutException e) { - result.hits().addError(ErrorMessage.createTimeout("timeout waiting for summaries from " + getName())); - return; - } catch (IOException e) { - result.hits().addError(ErrorMessage.createBackendCommunicationError( - "IO error while talking on channel " + getName() + " (summary fetch): " + e.getMessage())); - return; - } - if (receivedPackets.length == 0) { - result.hits().addError(ErrorMessage.createBackendCommunicationError(getName() + " got no packets back (summary fetch)")); - return; - } - } - } else { - packetKeys = new DocsumPacketKey[0]; - receivedPackets = new Packet[0]; - } - - int skippedHits; - try { - FillHitsResult fillHitsResult = fillHits(result, receivedPackets, summaryClass); - skippedHits = fillHitsResult.skippedHits; - if (fillHitsResult.error != null) { - result.hits().addError(ErrorMessage.createTimeout(fillHitsResult.error)); - return; - } - } catch (TimeoutException e) { - result.hits().addError(ErrorMessage.createTimeout(e.getMessage())); - return; - } catch (IOException e) { - result.hits().addError(ErrorMessage.createBackendCommunicationError("Error filling hits with summary fields, source: " + getName() + " Exception thrown: " + e.getMessage())); - return; - } - if (skippedHits == 0 && packetWrapper != null) { - cacheControl.updateCacheEntry(cacheKey, query, packetKeys, receivedPackets); - } - - if ( skippedHits > 0 ) - result.hits().addError(com.yahoo.search.result.ErrorMessage.createEmptyDocsums("Missing hit data for summary '" + summaryClass + "' for " + skippedHits + " hits")); - result.analyzeHits(); - - if (query.getTraceLevel() >= 3) { - int hitNumber = 0; - for (Iterator<com.yahoo.search.result.Hit> i = hitIterator(result); i.hasNext();) { - com.yahoo.search.result.Hit hit = i.next(); - if ( ! (hit instanceof FastHit)) continue; - FastHit fastHit = (FastHit) hit; - - String traceMsg = "Hit: " + (hitNumber++) + " from " + (fastHit.isCached() ? "cache" : "backend" ); - if ( ! fastHit.isFilled(summaryClass)) - traceMsg += ". Error, hit, not filled"; - query.trace(traceMsg, false, 3); - } - } + try (CloseableChannel channel = getChannel(query)) { + channel.partialFill(result, summaryClass); } } @@ -362,158 +266,10 @@ public class FastSearcher extends VespaBackEndSearcher { return Optional.of(summaryClass == null ? "[null]" : quote(summaryClass)); } - private CacheKey fetchCacheKeyFromHits(HitGroup hits, String summaryClass) { - for (Iterator<Hit> i = hits.unorderedDeepIterator(); i.hasNext();) { - Hit h = i.next(); - if (h instanceof FastHit) { - FastHit hit = (FastHit) h; - if (hit.isFilled(summaryClass)) { - continue; - } - if (hit.getCacheKey() != null) { - return hit.getCacheKey(); - } - } - } - return null; - } - - private Result searchTwoPhase(CloseableChannel channel, Query query, QueryPacket queryPacket, CacheKey cacheKey) throws IOException { - if (isLoggingFine()) - getLogger().finest("sending query packet"); - - try { - boolean couldSend = channel.sendPacket(queryPacket); - if ( ! couldSend) - return new Result(query,ErrorMessage.createBackendCommunicationError("Could not reach '" + getName() + "'")); - } catch (InvalidChannelException e) { - return new Result(query,ErrorMessage.createBackendCommunicationError("Invalid channel " + getName())); - } catch (IllegalStateException e) { - return new Result(query, ErrorMessage.createBackendCommunicationError("Illegal state in FS4: " + e.getMessage())); - } - - BasicPacket[] basicPackets; - - try { - basicPackets = channel.receivePackets(query.getTimeLeft(), 1); - } catch (ChannelTimeoutException e) { - return new Result(query,ErrorMessage.createTimeout("Timeout while waiting for " + getName())); - } catch (InvalidChannelException e) { - return new Result(query,ErrorMessage.createBackendCommunicationError("Invalid channel for " + getName())); - } - - if (basicPackets.length == 0) { - return new Result(query,ErrorMessage.createBackendCommunicationError(getName() + " got no packets back")); - } - - if (isLoggingFine()) - getLogger().finest("got packets " + basicPackets.length + " packets"); - - ensureInstanceOf(QueryResultPacket.class, basicPackets[0], getName()); - QueryResultPacket resultPacket = (QueryResultPacket) basicPackets[0]; - - if (isLoggingFine()) - getLogger().finest("got query packet. " + "docsumClass=" + query.getPresentation().getSummary()); - - if (query.getPresentation().getSummary() == null) - query.getPresentation().setSummary(getDefaultDocsumClass()); - - Result result = new Result(query); - - addMetaInfo(query, queryPacket.getQueryPacketData(), resultPacket, result, false); - - addUnfilledHits(result, resultPacket.getDocuments(), false, - queryPacket.getQueryPacketData(), cacheKey, channel.distributionKey()); - Packet[] packets; - PacketWrapper packetWrapper = cacheControl.lookup(cacheKey, query); - - if (packetWrapper != null) { - cacheControl.updateCacheEntry(cacheKey, query, resultPacket); - } - else { - if (resultPacket.getCoverageFeature() && ! resultPacket.getCoverageFull()) { - // Don't add error here, it was done in first phase - // No check if packetWrapper already exists, since incomplete - // first phase data won't be cached anyway. - } else { - packets = new Packet[1]; - packets[0] = resultPacket; - cacheControl.cache(cacheKey, query, new DocsumPacketKey[0], packets, channel.distributionKey()); - } - } - return result; - } - - private Packet[] convertBasicPackets(BasicPacket[] basicPackets) throws ClassCastException { - // trying to cast a BasicPacket[] to Packet[] will compile, - // but lead to a runtime error. At least that's what I got - // from testing and reading the specification. I'm just happy - // if someone tells me what's the proper Java way of doing - // this. -SK - Packet[] packets = new Packet[basicPackets.length]; - - for (int i = 0; i < basicPackets.length; i++) { - packets[i] = (Packet) basicPackets[i]; - } - return packets; - } - - private Packet[] fetchSummaries(CloseableChannel channel, Result result, String summaryClass) - throws InvalidChannelException, ChannelTimeoutException, ClassCastException, IOException { - - BasicPacket[] receivedPackets; - boolean summaryNeedsQuery = summaryNeedsQuery(result.getQuery()); - if (result.getQuery().getTraceLevel() >=3) - result.getQuery().trace((summaryNeedsQuery ? "Resending " : "Not resending ") + "query during document summary fetching", 3); - - GetDocSumsPacket docsumsPacket = GetDocSumsPacket.create(result, summaryClass, summaryNeedsQuery); - int compressionLimit = result.getQuery().properties().getInteger(PACKET_COMPRESSION_LIMIT, 0); - docsumsPacket.setCompressionLimit(compressionLimit); - if (compressionLimit != 0) { - docsumsPacket.setCompressionType(result.getQuery().properties().getString(PACKET_COMPRESSION_TYPE, "lz4")); - } - - boolean couldSend = channel.sendPacket(docsumsPacket); - if ( ! couldSend) throw new IOException("Could not successfully send GetDocSumsPacket."); - receivedPackets = channel.receivePackets(result.getQuery().getTimeLeft(), docsumsPacket.getNumDocsums() + 1); - - return convertBasicPackets(receivedPackets); - } - public String toString() { return "fast searcher (" + getName() + ") " + dispatchBackend; } - /** - * Returns an array of the hits contained in this result - * - * @param filled true to return all hits, false to return only unfilled hits - * @return array of docids, empty array if no hits - */ - private DocsumPacketKey[] getPacketKeys(Result result, String summaryClass, boolean filled) { - DocsumPacketKey[] packetKeys = new DocsumPacketKey[result.getHitCount()]; - int x = 0; - - for (Iterator<com.yahoo.search.result.Hit> i = hitIterator(result); i.hasNext();) { - com.yahoo.search.result.Hit hit = i.next(); - if (hit instanceof FastHit) { - FastHit fastHit = (FastHit) hit; - if(filled || !fastHit.isFilled(summaryClass)) { - packetKeys[x] = new DocsumPacketKey(fastHit.getGlobalId(), fastHit.getPartId(), summaryClass); - x++; - } - } - } - if (x < packetKeys.length) { - DocsumPacketKey[] tmp = new DocsumPacketKey[x]; - - System.arraycopy(packetKeys, 0, tmp, 0, x); - return tmp; - } else { - return packetKeys; - } - } - protected boolean isLoggingFine() { return getLogger().isLoggable(Level.FINE); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java index 3e9a92ea0f7..a6f98418a76 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java @@ -1,15 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.fastsearch; -import java.util.Optional; import com.yahoo.collections.TinyIdentitySet; -import com.yahoo.fs4.BasicPacket; import com.yahoo.fs4.DocsumPacket; import com.yahoo.fs4.DocumentInfo; -import com.yahoo.fs4.ErrorPacket; -import com.yahoo.fs4.QueryPacketData; import com.yahoo.fs4.Packet; import com.yahoo.fs4.QueryPacket; +import com.yahoo.fs4.QueryPacketData; import com.yahoo.fs4.QueryResultPacket; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.io.HexDump; @@ -42,7 +39,9 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.logging.Level; +import java.util.logging.Logger; /** @@ -53,8 +52,8 @@ import java.util.logging.Level; @SuppressWarnings("deprecation") public abstract class VespaBackEndSearcher extends PingableSearcher { - protected static final CompoundName PACKET_COMPRESSION_LIMIT = new CompoundName("packetcompressionlimit"); - protected static final CompoundName PACKET_COMPRESSION_TYPE = new CompoundName("packetcompressiontype"); + static final CompoundName PACKET_COMPRESSION_LIMIT = new CompoundName("packetcompressionlimit"); + static final CompoundName PACKET_COMPRESSION_TYPE = new CompoundName("packetcompressiontype"); protected static final CompoundName TRACE_DISABLE = new CompoundName("trace.disable"); /** The set of all document databases available in the backend handled by this searcher */ @@ -65,7 +64,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { private String defaultDocsumClass = null; /** Returns an iterator which returns all hits below this result **/ - protected Iterator<Hit> hitIterator(Result result) { + static Iterator<Hit> hitIterator(Result result) { return result.hits().unorderedDeepIterator(); } @@ -75,7 +74,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { /** Cache wrapper */ protected CacheControl cacheControl = null; - protected final String getName() { return name; } + public final String getName() { return name; } protected final String getDefaultDocsumClass() { return defaultDocsumClass; } /** Sets default document summary class. Default is null */ @@ -84,6 +83,8 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { /** Returns the packet cache controller of this */ public final CacheControl getCacheControl() { return cacheControl; } + public final Logger getLogger() { return super.getLogger(); } + /** * Searches a search cluster * This is an endpoint - searchers will never propagate the search to any nested searcher. @@ -101,7 +102,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { * Returns whether we need to send the query when fetching summaries. * This is necessary if the query requests summary features or dynamic snippeting */ - protected boolean summaryNeedsQuery(Query query) { + boolean summaryNeedsQuery(Query query) { if (query.getRanking().getQueryCache()) return false; // Query is cached in backend DocumentDatabase documentDb = getDocumentDatabase(query); @@ -135,7 +136,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { Result result = new Result(query); QueryResultPacket resultPacket = packetWrapper.getFirstResultPacket(); - addMetaInfo(query, queryPacketData, resultPacket, result, true); + addMetaInfo(query, queryPacketData, resultPacket, result); if (packetWrapper.getNumPackets() == 0) addUnfilledHits(result, documents, true, queryPacketData, key, packetWrapper.distributionKey()); else @@ -400,7 +401,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { } } - protected void addMetaInfo(Query query, QueryPacketData queryPacketData, QueryResultPacket resultPacket, Result result, boolean fromCache) { + void addMetaInfo(Query query, QueryPacketData queryPacketData, QueryResultPacket resultPacket, Result result) { result.setTotalHitCount(resultPacket.getTotalDocumentCount()); // Grouping @@ -429,7 +430,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { } } - static private class FillHitResult { + static class FillHitResult { final boolean ok; final String error; FillHitResult(boolean ok) { @@ -440,7 +441,8 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { this.error = error; } } - private FillHitResult fillHit(FastHit hit, DocsumPacket packet, String summaryClass) { + + FillHitResult fillHit(FastHit hit, DocsumPacket packet, String summaryClass) { if (packet != null) { byte[] docsumdata = packet.getData(); if (docsumdata.length > 0) { @@ -464,7 +466,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { * @return the number of hits that we did not return data for, and an optional error message. * when things are working normally we return 0. */ - protected FillHitsResult fillHits(Result result, Packet[] packets, String summaryClass) throws IOException { + public FillHitsResult fillHits(Result result, Packet[] packets, String summaryClass) throws IOException { int skippedHits = 0; String lastError = null; int packetIndex = 0; @@ -474,7 +476,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { if (hit instanceof FastHit && ! hit.isFilled(summaryClass)) { FastHit fastHit = (FastHit) hit; - ensureInstanceOf(DocsumPacket.class, packets[packetIndex], getName()); + packets[packetIndex].ensureInstanceOf(DocsumPacket.class, getName()); DocsumPacket docsum = (DocsumPacket) packets[packetIndex]; packetIndex++; @@ -493,23 +495,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new FillHitsResult(skippedHits, lastError); } - /** - * Throws an IOException if the packet is not of the expected type - */ - protected static void ensureInstanceOf(Class<? extends BasicPacket> type, BasicPacket packet, String name) throws IOException { - if ((type.isAssignableFrom(packet.getClass()))) return; - - if (packet instanceof ErrorPacket) { - ErrorPacket errorPacket=(ErrorPacket)packet; - if (errorPacket.getErrorCode() == 8) - throw new TimeoutException("Query timed out in " + name); - else - throw new IOException("Received error from backend in " + name + ": " + packet); - } else { - throw new IOException("Received " + packet + " when expecting " + type); - } - } - private boolean addCachedHits(Result result, PacketWrapper packetWrapper, String summaryClass, @@ -562,34 +547,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { hit.setPartId(document.getPartId()); } - protected PacketWrapper cacheLookupTwoPhase(CacheKey cacheKey, Result result, String summaryClass) { - Query query = result.getQuery(); - PacketWrapper packetWrapper = cacheControl.lookup(cacheKey, query); - - if (packetWrapper == null) { - return null; - } - if (packetWrapper.getNumPackets() != 0) { - for (Iterator<Hit> i = hitIterator(result); i.hasNext();) { - Hit hit = i.next(); - - if (hit instanceof FastHit) { - FastHit fastHit = (FastHit) hit; - DocsumPacketKey key = new DocsumPacketKey(fastHit.getGlobalId(), fastHit.getPartId(), summaryClass); - - if (fillHit(fastHit, (DocsumPacket) packetWrapper.getPacket(key), summaryClass).ok) { - fastHit.setCached(true); - } - - } - } - result.hits().setSorted(false); - result.analyzeHits(); - } - - return packetWrapper; - } - protected DocsumDefinitionSet getDocsumDefinitionSet(Query query) { DocumentDatabase db = getDocumentDatabase(query); return db.getDocsumDefinitionSet(); @@ -620,13 +577,12 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { * Only set if produced directly by a search node, not dispatch * (in which case it is not set in the received packets.) */ - boolean addUnfilledHits(Result result, + void addUnfilledHits(Result result, List<DocumentInfo> documents, boolean fromCache, QueryPacketData queryPacketData, CacheKey cacheKey, Optional<Integer> channelDistributionKey) { - boolean allHitsOK = true; Query myQuery = result.getQuery(); for (DocumentInfo document : documents) { @@ -646,14 +602,11 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { result.hits().add(hit); } catch (ConfigurationException e) { - allHitsOK = false; getLogger().log(LogLevel.WARNING, "Skipping hit", e); } catch (Exception e) { - allHitsOK = false; getLogger().log(LogLevel.ERROR, "Skipping malformed hit", e); } } - return allHitsOK; } @SuppressWarnings("rawtypes") diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/CloseableChannel.java b/container-search/src/main/java/com/yahoo/search/dispatch/CloseableChannel.java index 643b8f81318..3f5ebe53d0d 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/CloseableChannel.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/CloseableChannel.java @@ -1,54 +1,41 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.BasicPacket; -import com.yahoo.fs4.ChannelTimeoutException; -import com.yahoo.fs4.mplex.Backend; -import com.yahoo.fs4.mplex.FS4Channel; -import com.yahoo.fs4.mplex.InvalidChannelException; +import com.yahoo.fs4.QueryPacket; +import com.yahoo.prelude.fastsearch.CacheKey; import com.yahoo.search.Query; +import com.yahoo.search.Result; import java.io.Closeable; import java.io.IOException; -import java.util.Optional; /** + * CloseableChannel is an interface for running a search query and getting document summaries against some + * content node, node group or dispatcher while abstracting the specifics of the invocation target. + * * @author ollivir */ -public class CloseableChannel implements Closeable { - private FS4Channel channel; - private final Optional<Integer> distributionKey; +public abstract class CloseableChannel implements Closeable { + /** Retrieve the hits for the given {@link Query} */ + public abstract Result search(Query query, QueryPacket queryPacket, CacheKey cacheKey) throws IOException; - public CloseableChannel(Backend backend) { - this(backend, Optional.empty()); - } - - public CloseableChannel(Backend backend, Optional<Integer> distributionKey) { - this.channel = backend.openChannel(); - this.distributionKey = distributionKey; - } + /** Retrieve document summaries for the unfilled hits in the given {@link Result} */ + public abstract void partialFill(Result result, String summaryClass); - public void setQuery(Query query) { - channel.setQuery(query); - } + protected abstract void closeChannel(); - public boolean sendPacket(BasicPacket packet) throws InvalidChannelException, IOException { - return channel.sendPacket(packet); - } - - public BasicPacket[] receivePackets(long timeout, int packetCount) throws InvalidChannelException, ChannelTimeoutException { - return channel.receivePackets(timeout, packetCount); - } + private Runnable teardown = null; - public Optional<Integer> distributionKey() { - return distributionKey; + public void teardown(Runnable teardown) { + this.teardown = teardown; } @Override - public void close() { - if (channel != null) { - channel.close(); - channel = null; + public final void close() { + if (teardown != null) { + teardown.run(); + teardown = null; } + closeChannel(); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/DispatchedChannel.java b/container-search/src/main/java/com/yahoo/search/dispatch/DispatchedChannel.java deleted file mode 100644 index d005d9491d5..00000000000 --- a/container-search/src/main/java/com/yahoo/search/dispatch/DispatchedChannel.java +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.search.dispatch; - -import com.yahoo.prelude.fastsearch.FS4ResourcePool; -import com.yahoo.search.dispatch.SearchCluster.Group; -import com.yahoo.search.dispatch.SearchCluster.Node; - -import java.util.Optional; - -/** - * An extension to CloseableChannel that encapsulates the release of a LoadBalancer group allocation. - * - * @author ollivir - */ -public class DispatchedChannel extends CloseableChannel { - private final SearchCluster.Group group; - private final LoadBalancer loadBalancer; - private boolean groupAllocated = true; - - public DispatchedChannel(FS4ResourcePool fs4ResourcePool, LoadBalancer loadBalancer, Group group, Node node) { - super(fs4ResourcePool.getBackend(node.hostname(), node.fs4port()), Optional.of(node.key())); - - this.loadBalancer = loadBalancer; - this.group = group; - } - - public DispatchedChannel(FS4ResourcePool fs4ResourcePool, LoadBalancer loadBalancer, Group group) { - this(fs4ResourcePool, loadBalancer, group, group.nodes().iterator().next()); - } - - public void close() { - if (groupAllocated) { - groupAllocated = false; - loadBalancer.releaseGroup(group); - } - super.close(); - } -} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index c383b681558..0cf18852dd3 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -8,18 +8,20 @@ import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; import com.yahoo.container.handler.VipStatus; import com.yahoo.container.protect.Error; -import com.yahoo.prelude.fastsearch.DocumentDatabase; -import com.yahoo.slime.ArrayTraverser; +import com.yahoo.data.access.Inspector; import com.yahoo.data.access.slime.SlimeAdapter; +import com.yahoo.prelude.fastsearch.DocumentDatabase; +import com.yahoo.prelude.fastsearch.FS4CloseableChannel; import com.yahoo.prelude.fastsearch.FS4ResourcePool; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.TimeoutException; +import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.query.SessionId; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Hit; -import com.yahoo.data.access.Inspector; +import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.BinaryFormat; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; @@ -52,7 +54,7 @@ public class Dispatcher extends AbstractComponent { /** A model of the search cluster this dispatches to */ private final SearchCluster searchCluster; - + /** Connections to the search nodes this talks to, indexed by node id ("partid") */ private final ImmutableMap<Integer, Client.NodeConnection> nodeConnections; @@ -84,7 +86,7 @@ public class Dispatcher extends AbstractComponent { this.fs4ResourcePool = null; this.loadBalancer = new LoadBalancer(searchCluster); } - + /** Returns the search cluster this dispatches to */ public SearchCluster searchCluster() { return searchCluster; } @@ -283,14 +285,18 @@ public class Dispatcher extends AbstractComponent { } - public Optional<CloseableChannel> getDispatchedChannel(Query query) { + public Optional<CloseableChannel> getDispatchedChannel(VespaBackEndSearcher searcher, Query query) { Optional<SearchCluster.Group> groupInCluster = loadBalancer.takeGroupForQuery(query); return groupInCluster.flatMap(group -> { if(group.nodes().size() == 1) { SearchCluster.Node node = group.nodes().iterator().next(); query.trace(false, 2, "Dispatching internally to ", group, " (", node.toString(), ")"); - return Optional.of(new DispatchedChannel(fs4ResourcePool, loadBalancer, group)); + CloseableChannel channel = new FS4CloseableChannel(searcher, query, fs4ResourcePool, node.hostname(), node.fs4port(), node.key()); + channel.teardown(() -> { + loadBalancer.releaseGroup(group); + }); + return Optional.of(channel); } else { loadBalancer.releaseGroup(group); return Optional.empty(); diff --git a/document/src/vespa/document/datatype/datatype.h b/document/src/vespa/document/datatype/datatype.h index 0e444ce8d3b..4dd5d6aae64 100644 --- a/document/src/vespa/document/datatype/datatype.h +++ b/document/src/vespa/document/datatype/datatype.h @@ -46,7 +46,7 @@ protected: explicit DataType(vespalib::stringref name); public: - virtual ~DataType(); + ~DataType() override; typedef std::unique_ptr<DataType> UP; typedef std::shared_ptr<DataType> SP; typedef vespalib::CloneablePtr<DataType> CP; diff --git a/document/src/vespa/document/datatype/referencedatatype.cpp b/document/src/vespa/document/datatype/referencedatatype.cpp index 7b7c83c7fa6..bc91f6b30ed 100644 --- a/document/src/vespa/document/datatype/referencedatatype.cpp +++ b/document/src/vespa/document/datatype/referencedatatype.cpp @@ -15,8 +15,7 @@ ReferenceDataType::ReferenceDataType(const DocumentType& targetDocType, int id) { } -ReferenceDataType::~ReferenceDataType() { -} +ReferenceDataType::~ReferenceDataType() = default; std::unique_ptr<FieldValue> ReferenceDataType::createFieldValue() const { return std::make_unique<ReferenceFieldValue>(*this); @@ -25,8 +24,7 @@ std::unique_ptr<FieldValue> ReferenceDataType::createFieldValue() const { void ReferenceDataType::print(std::ostream& os, bool verbose, const std::string& indent) const { (void) verbose; (void) indent; - os << "ReferenceDataType(" << _targetDocType.getName() - << ", id " << getId() << ')'; + os << "ReferenceDataType(" << _targetDocType.getName() << ", id " << getId() << ')'; } ReferenceDataType* ReferenceDataType::clone() const { diff --git a/document/src/vespa/document/datatype/referencedatatype.h b/document/src/vespa/document/datatype/referencedatatype.h index 5ca52f3ccb2..bc7db7800aa 100644 --- a/document/src/vespa/document/datatype/referencedatatype.h +++ b/document/src/vespa/document/datatype/referencedatatype.h @@ -14,7 +14,7 @@ class ReferenceDataType : public DataType { const DocumentType& _targetDocType; public: ReferenceDataType(const DocumentType& targetDocType, int id); - ~ReferenceDataType(); + ~ReferenceDataType() override; const DocumentType& getTargetType() const noexcept { return _targetDocType; diff --git a/model-evaluation/pom.xml b/model-evaluation/pom.xml index 328d475c501..7c7410df833 100644 --- a/model-evaluation/pom.xml +++ b/model-evaluation/pom.xml @@ -72,6 +72,18 @@ <artifactId>guava</artifactId> <scope>provided</scope> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>jdisc_http_service</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>jdisc_jetty</artifactId> + <version>${project.version}</version> + <scope>test</scope> + </dependency> </dependencies> <build> <plugins> diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java index e08b9f77d15..1412936d4a0 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java @@ -56,6 +56,6 @@ public class FunctionEvaluator { return function.getBody().evaluate(context).asTensor(); } - LazyArrayContext context() { return context; } + public LazyArrayContext context() { return context; } } diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java index beaa36b898f..c7d0cbd8f30 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java @@ -26,7 +26,7 @@ import java.util.Set; * * @author bratseth */ -final class LazyArrayContext extends Context implements ContextIndex { +public final class LazyArrayContext extends Context implements ContextIndex { private final IndexedBindings indexedBindings; diff --git a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java index 78c46864d7b..1c995c255f5 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java +++ b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java @@ -1,47 +1,213 @@ package ai.vespa.models.handler; +import ai.vespa.models.evaluation.FunctionEvaluator; +import ai.vespa.models.evaluation.Model; import ai.vespa.models.evaluation.ModelsEvaluator; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.jdisc.LoggingRequestHandler; +import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; +import com.yahoo.searchlib.rankingexpression.ExpressionFunction; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.serialization.JsonFormat; import java.io.IOException; import java.io.OutputStream; +import java.net.URI; +import java.nio.charset.Charset; +import java.util.Optional; +import java.util.concurrent.Executor; -public class ModelsEvaluationHandler extends LoggingRequestHandler { +public class ModelsEvaluationHandler extends ThreadedHttpRequestHandler { + + public static final String API_ROOT = "model-evaluation"; + public static final String VERSION_V1 = "v1"; + public static final String EVALUATE = "eval"; private final ModelsEvaluator modelsEvaluator; - public ModelsEvaluationHandler(ModelsEvaluator modelsEvaluator, Context context) { - super(context); + public ModelsEvaluationHandler(ModelsEvaluator modelsEvaluator, Executor executor) { + super(executor); this.modelsEvaluator = modelsEvaluator; } @Override public HttpResponse handle(HttpRequest request) { - Tensor result = modelsEvaluator.evaluatorOf(property("model", "serving_default", request), - request.getProperty("function")) - .evaluate(); - return new RawResponse(JsonFormat.encode(result)); + Path path = new Path(request); + Optional<String> apiName = path.segment(0); + Optional<String> version = path.segment(1); + Optional<String> modelName = path.segment(2); + + if ( ! apiName.isPresent() || ! apiName.get().equalsIgnoreCase(API_ROOT)) { + return new ErrorResponse(404, "unknown API"); + } + if ( ! version.isPresent() || ! version.get().equalsIgnoreCase(VERSION_V1)) { + return new ErrorResponse(404, "unknown API version"); + } + if ( ! modelName.isPresent()) { + return listAllModels(request); + } + if ( ! modelsEvaluator.models().containsKey(modelName.get())) { + // TODO: Replace by catching IllegalArgumentException and passing that error message + return new ErrorResponse(404, "no model with name '" + modelName.get() + "' found"); + } + + Model model = modelsEvaluator.models().get(modelName.get()); + + // The following logic follows from the spec, in that signature and + // output are optional if the model only has a single function. + // TODO: Try to avoid recreating that logic here + + if (path.segments() == 3) { + if (model.functions().size() > 1) { + return listModelDetails(request, modelName.get()); + } + return listTypeDetails(request, modelName.get()); + } + + if (path.segments() == 4) { + if ( ! path.segment(3).get().equalsIgnoreCase(EVALUATE)) { + return listTypeDetails(request, modelName.get(), path.segment(3).get()); + } + if (model.functions().stream().anyMatch(f -> f.getName().equalsIgnoreCase(EVALUATE))) { + return listTypeDetails(request, modelName.get(), path.segment(3).get()); // model has a function "eval" + } + if (model.functions().size() <= 1) { + return evaluateModel(request, modelName.get()); + } + // TODO: Replace by catching IllegalArgumentException and passing that error message + return new ErrorResponse(404, "attempt to evaluate model without specifying function"); + } + + if (path.segments() == 5) { + if (path.segment(4).get().equalsIgnoreCase(EVALUATE)) { + return evaluateModel(request, modelName.get(), path.segment(3).get()); + } + } + + return new ErrorResponse(404, "unrecognized request"); + } + + private HttpResponse listAllModels(HttpRequest request) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + for (String modelName: modelsEvaluator.models().keySet()) { + root.setString(modelName, baseUrl(request) + modelName); + } + return new Response(200, com.yahoo.slime.JsonFormat.toJsonBytes(slime)); + } + + private HttpResponse listModelDetails(HttpRequest request, String modelName) { + Model model = modelsEvaluator.models().get(modelName); + Slime slime = new Slime(); + Cursor root = slime.setObject(); + for (ExpressionFunction func : model.functions()) { + root.setString(func.getName(), baseUrl(request) + modelName + "/" + func.getName()); + } + return new Response(200, com.yahoo.slime.JsonFormat.toJsonBytes(slime)); + } + + private HttpResponse listTypeDetails(HttpRequest request, String modelName) { + return listTypeDetails(request, modelsEvaluator.evaluatorOf(modelName)); + } + + private HttpResponse listTypeDetails(HttpRequest request, String modelName, String signatureAndOutput) { + return listTypeDetails(request, modelsEvaluator.evaluatorOf(modelName, signatureAndOutput)); + } + + private HttpResponse listTypeDetails(HttpRequest request, FunctionEvaluator evaluator) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + Cursor bindings = root.setArray("bindings"); + for (String bindingName : evaluator.context().names()) { + // TODO: Use an API which exposes only the external binding names instead of this + if (bindingName.startsWith("constant(")) { + continue; + } + if (bindingName.startsWith("rankingExpression(")) { + continue; + } + Cursor binding = bindings.addObject(); + binding.setString("name", bindingName); + binding.setString("type", ""); // TODO: implement type information when available + } + return new Response(200, com.yahoo.slime.JsonFormat.toJsonBytes(slime)); + } + + private HttpResponse evaluateModel(HttpRequest request, String modelName) { + return evaluateModel(request, modelsEvaluator.evaluatorOf(modelName)); + } + + private HttpResponse evaluateModel(HttpRequest request, String modelName, String signatureAndOutput) { + return evaluateModel(request, modelsEvaluator.evaluatorOf(modelName, signatureAndOutput)); + } + + private HttpResponse evaluateModel(HttpRequest request, FunctionEvaluator evaluator) { + for (String bindingName : evaluator.context().names()) { + property(request, bindingName).ifPresent(s -> evaluator.bind(bindingName, Tensor.from(s))); + } + Tensor result = evaluator.evaluate(); + return new Response(200, JsonFormat.encode(result)); + } + + private Optional<String> property(HttpRequest request, String name) { + return Optional.ofNullable(request.getProperty(name)); } - private String property(String name, String defaultValue, HttpRequest request) { - String value = request.getProperty(name); - if (value == null) return defaultValue; - return value; + private String baseUrl(HttpRequest request) { + URI uri = request.getUri(); + StringBuilder sb = new StringBuilder(); + sb.append(uri.getScheme()).append("://").append(uri.getHost()); + if (uri.getPort() >= 0) { + sb.append(":").append(uri.getPort()); + } + sb.append("/").append(API_ROOT).append("/").append(VERSION_V1).append("/"); + return sb.toString(); } - private static class RawResponse extends HttpResponse { + private static class Path { + + private final String[] segments; + + public Path(HttpRequest httpRequest) { + segments = splitPath(httpRequest); + } + + Optional<String> segment(int index) { + return (index < 0 || index >= segments.length) ? Optional.empty() : Optional.of(segments[index]); + } + + int segments() { + return segments.length; + } + + private static String[] splitPath(HttpRequest request) { + String path = request.getUri().getPath().toLowerCase(); + if (path.startsWith("/")) { + path = path.substring("/".length()); + } + if (path.endsWith("/")) { + path = path.substring(0, path.length() - 1); + } + return path.split("/"); + } + + } + + private static class Response extends HttpResponse { private final byte[] data; - RawResponse(byte[] data) { - super(200); + Response(int code, byte[] data) { + super(code); this.data = data; } + Response(int code, String data) { + this(code, data.getBytes(Charset.forName(DEFAULT_CHARACTER_ENCODING))); + } + @Override public String getContentType() { return "application/json"; @@ -53,5 +219,11 @@ public class ModelsEvaluationHandler extends LoggingRequestHandler { } } + private static class ErrorResponse extends Response { + ErrorResponse(int code, String data) { + super(code, "{\"error\":\"" + data + "\"}"); + } + } + } diff --git a/model-evaluation/src/main/java/ai/vespa/models/handler/package-info.java b/model-evaluation/src/main/java/ai/vespa/models/handler/package-info.java new file mode 100644 index 00000000000..7978abf2632 --- /dev/null +++ b/model-evaluation/src/main/java/ai/vespa/models/handler/package-info.java @@ -0,0 +1,4 @@ +@ExportPackage +package ai.vespa.models.handler; + +import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelTester.java b/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelTester.java index 0aceaccc3e0..9a3e59aed80 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelTester.java +++ b/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelTester.java @@ -65,7 +65,7 @@ public class ModelTester { } /** Allows us to provide canned tensor constants during import since file distribution does not work in tests */ - private static class RankProfilesConfigImporterWithMockedConstants extends RankProfilesConfigImporter { + public static class RankProfilesConfigImporterWithMockedConstants extends RankProfilesConfigImporter { private static final Logger log = Logger.getLogger(RankProfilesConfigImporterWithMockedConstants.class.getName()); diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java new file mode 100644 index 00000000000..5f045a2feb4 --- /dev/null +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java @@ -0,0 +1,201 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package ai.vespa.models.handler; + +import ai.vespa.models.evaluation.ModelTester; +import ai.vespa.models.evaluation.ModelsEvaluator; +import com.yahoo.config.subscription.ConfigGetter; +import com.yahoo.config.subscription.FileSource; +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.filedistribution.fileacquirer.MockFileAcquirer; +import com.yahoo.path.Path; +import com.yahoo.vespa.config.search.RankProfilesConfig; +import com.yahoo.vespa.config.search.core.RankingConstantsConfig; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; + +import static org.junit.Assert.assertEquals; + +public class ModelsEvaluationHandlerTest { + + private static ModelsEvaluationHandler handler; + + @BeforeClass + static public void setUp() { + Executor executor = Executors.newSingleThreadExecutor(); + ModelsEvaluator models = createModels("src/test/resources/config/models/"); + handler = new ModelsEvaluationHandler(models, executor); + } + + @Test + public void testUnknownAPI() { + assertResponse("http://localhost/wrong-api-binding", 404); + } + + @Test + public void testUnknownVersion() { + assertResponse("http://localhost/model-evaluation/v0", 404); + } + + @Test + public void testNonExistingModel() { + assertResponse("http://localhost/model-evaluation/v1/non-existing-model", 404); + } + + @Test + public void testListModels() { + String url = "http://localhost/model-evaluation/v1"; + String expected = "{\"mnist_softmax\":\"http://localhost/model-evaluation/v1/mnist_softmax\",\"mnist_saved\":\"http://localhost/model-evaluation/v1/mnist_saved\",\"mnist_softmax_saved\":\"http://localhost/model-evaluation/v1/mnist_softmax_saved\",\"xgboost_2_2\":\"http://localhost/model-evaluation/v1/xgboost_2_2\"}"; + assertResponse(url, 200, expected); + } + + @Test + public void testXgBoostEvaluationWithoutBindings() { + String url = "http://localhost/model-evaluation/v1/xgboost_2_2/eval"; // only has a single function + String expected = "{\"cells\":[{\"address\":{},\"value\":-8.17695}]}"; + assertResponse(url, 200, expected); + } + + @Test + public void testXgBoostEvaluationWithBindings() { + Map<String, String> properties = new HashMap<>(); + properties.put("f29", "-1.0"); + properties.put("f56", "0.2"); + properties.put("f60", "0.3"); + properties.put("f109", "0.4"); + properties.put("non-existing-binding", "-1"); + String url = "http://localhost/model-evaluation/v1/xgboost_2_2/eval"; + String expected = "{\"cells\":[{\"address\":{},\"value\":-7.936679999999999}]}"; + assertResponse(url, properties, 200, expected); + } + + @Test + public void testMnistSoftmaxDetails() { + String url = "http://localhost:8080/model-evaluation/v1/mnist_softmax"; + String expected = "{\"bindings\":[{\"name\":\"Placeholder\",\"type\":\"\"}]}"; // only has a single function + assertResponse(url, 200, expected); + } + + @Test + public void testMnistSoftmaxTypeDetails() { + String url = "http://localhost/model-evaluation/v1/mnist_softmax/default.add/"; + String expected = "{\"bindings\":[{\"name\":\"Placeholder\",\"type\":\"\"}]}"; + assertResponse(url, 200, expected); + } + + @Test + public void testMnistSoftmaxEvaluateDefaultFunctionWithoutBindings() { + String url = "http://localhost/model-evaluation/v1/mnist_softmax/eval"; + String expected = "{\"cells\":[{\"address\":{\"d1\":\"0\"},\"value\":-0.3546536862850189},{\"address\":{\"d1\":\"1\"},\"value\":0.3759574592113495},{\"address\":{\"d1\":\"2\"},\"value\":0.06054411828517914},{\"address\":{\"d1\":\"3\"},\"value\":-0.251544713973999},{\"address\":{\"d1\":\"4\"},\"value\":0.017951013520359993},{\"address\":{\"d1\":\"5\"},\"value\":1.2899067401885986},{\"address\":{\"d1\":\"6\"},\"value\":-0.10389615595340729},{\"address\":{\"d1\":\"7\"},\"value\":0.6367976665496826},{\"address\":{\"d1\":\"8\"},\"value\":-1.4136744737625122},{\"address\":{\"d1\":\"9\"},\"value\":-0.2573896050453186}]}"; + assertResponse(url, 200, expected); + } + + @Test + public void testMnistSoftmaxEvaluateSpecificFunctionWithoutBindings() { + String url = "http://localhost/model-evaluation/v1/mnist_softmax/default.add/eval"; + String expected = "{\"cells\":[{\"address\":{\"d1\":\"0\"},\"value\":-0.3546536862850189},{\"address\":{\"d1\":\"1\"},\"value\":0.3759574592113495},{\"address\":{\"d1\":\"2\"},\"value\":0.06054411828517914},{\"address\":{\"d1\":\"3\"},\"value\":-0.251544713973999},{\"address\":{\"d1\":\"4\"},\"value\":0.017951013520359993},{\"address\":{\"d1\":\"5\"},\"value\":1.2899067401885986},{\"address\":{\"d1\":\"6\"},\"value\":-0.10389615595340729},{\"address\":{\"d1\":\"7\"},\"value\":0.6367976665496826},{\"address\":{\"d1\":\"8\"},\"value\":-1.4136744737625122},{\"address\":{\"d1\":\"9\"},\"value\":-0.2573896050453186}]}"; + assertResponse(url, 200, expected); + } + + @Test + public void testMnistSoftmaxEvaluateDefaultFunctionWithBindings() { + Map<String, String> properties = new HashMap<>(); + properties.put("Placeholder", "{1.0}"); + String url = "http://localhost/model-evaluation/v1/mnist_softmax/eval"; + String expected = "{\"cells\":[{\"address\":{\"d1\":\"0\"},\"value\":2.7147769462592217},{\"address\":{\"d1\":\"1\"},\"value\":-19.710327346521872},{\"address\":{\"d1\":\"2\"},\"value\":9.496512226053643},{\"address\":{\"d1\":\"3\"},\"value\":13.11241075176957},{\"address\":{\"d1\":\"4\"},\"value\":-12.355567088005559},{\"address\":{\"d1\":\"5\"},\"value\":10.39812446509341},{\"address\":{\"d1\":\"6\"},\"value\":-1.3739236534397499},{\"address\":{\"d1\":\"7\"},\"value\":-3.4260787871386995},{\"address\":{\"d1\":\"8\"},\"value\":6.471120687192041},{\"address\":{\"d1\":\"9\"},\"value\":-5.327024804970982}]}"; + assertResponse(url, properties, 200, expected); + } + + @Test + public void testMnistSoftmaxEvaluateSpecificFunctionWithBindings() { + Map<String, String> properties = new HashMap<>(); + properties.put("Placeholder", "{1.0}"); + String url = "http://localhost/model-evaluation/v1/mnist_softmax/default.add/eval"; + String expected = "{\"cells\":[{\"address\":{\"d1\":\"0\"},\"value\":2.7147769462592217},{\"address\":{\"d1\":\"1\"},\"value\":-19.710327346521872},{\"address\":{\"d1\":\"2\"},\"value\":9.496512226053643},{\"address\":{\"d1\":\"3\"},\"value\":13.11241075176957},{\"address\":{\"d1\":\"4\"},\"value\":-12.355567088005559},{\"address\":{\"d1\":\"5\"},\"value\":10.39812446509341},{\"address\":{\"d1\":\"6\"},\"value\":-1.3739236534397499},{\"address\":{\"d1\":\"7\"},\"value\":-3.4260787871386995},{\"address\":{\"d1\":\"8\"},\"value\":6.471120687192041},{\"address\":{\"d1\":\"9\"},\"value\":-5.327024804970982}]}"; + assertResponse(url, properties, 200, expected); + } + + @Test + public void testMnistSavedDetails() { + String url = "http://localhost:8080/model-evaluation/v1/mnist_saved"; + String expected = "{\"imported_ml_macro_mnist_saved_dnn_hidden1_add\":\"http://localhost:8080/model-evaluation/v1/mnist_saved/imported_ml_macro_mnist_saved_dnn_hidden1_add\",\"serving_default.y\":\"http://localhost:8080/model-evaluation/v1/mnist_saved/serving_default.y\"}"; + assertResponse(url, 200, expected); + } + + @Test + public void testMnistSavedTypeDetails() { + String url = "http://localhost/model-evaluation/v1/mnist_saved/serving_default.y/"; + String expected = "{\"bindings\":[{\"name\":\"input\",\"type\":\"\"}]}"; + assertResponse(url, 200, expected); + } + + @Test + public void testMnistSavedEvaluateDefaultFunctionShouldFail() { + String url = "http://localhost/model-evaluation/v1/mnist_saved/eval"; + String expected = "{\"error\":\"attempt to evaluate model without specifying function\"}"; + assertResponse(url, 404, expected); + } + + @Test + public void testMnistSavedEvaluateSpecificFunction() { + Map<String, String> properties = new HashMap<>(); + properties.put("input", "-1.0"); + String url = "http://localhost/model-evaluation/v1/mnist_saved/serving_default.y/eval"; + String expected = "{\"cells\":[{\"address\":{\"d1\":\"0\"},\"value\":-2.72208123403445},{\"address\":{\"d1\":\"1\"},\"value\":6.465137496457595},{\"address\":{\"d1\":\"2\"},\"value\":-7.078050386283122},{\"address\":{\"d1\":\"3\"},\"value\":-10.485296462655546},{\"address\":{\"d1\":\"4\"},\"value\":0.19508378636937004},{\"address\":{\"d1\":\"5\"},\"value\":6.348870746681019},{\"address\":{\"d1\":\"6\"},\"value\":10.756191852397258},{\"address\":{\"d1\":\"7\"},\"value\":1.476101533270058},{\"address\":{\"d1\":\"8\"},\"value\":-17.778398655804875},{\"address\":{\"d1\":\"9\"},\"value\":-2.0597690508530295}]}"; + assertResponse(url, properties, 200, expected); + } + + static private void assertResponse(String url, int expectedCode) { + assertResponse(url, Collections.emptyMap(), expectedCode, null); + } + + static private void assertResponse(String url, int expectedCode, String expectedResult) { + assertResponse(url, Collections.emptyMap(), expectedCode, expectedResult); + } + + static private void assertResponse(String url, Map<String, String> properties, int expectedCode, String expectedResult) { + HttpRequest getRequest = HttpRequest.createTestRequest(url, com.yahoo.jdisc.http.HttpRequest.Method.GET, null, properties); + HttpRequest postRequest = HttpRequest.createTestRequest(url, com.yahoo.jdisc.http.HttpRequest.Method.POST, null, properties); + assertResponse(getRequest, expectedCode, expectedResult); + assertResponse(postRequest, expectedCode, expectedResult); + } + + static private void assertResponse(HttpRequest request, int expectedCode, String expectedResult) { + HttpResponse response = handler.handle(request); + assertEquals("application/json", response.getContentType()); + assertEquals(expectedCode, response.getStatus()); + if (expectedResult != null) { + assertEquals(expectedResult, getContents(response)); + } + } + + static private String getContents(HttpResponse response) { + try (ByteArrayOutputStream stream = new ByteArrayOutputStream()) { + response.render(stream); + return stream.toString(); + } catch (IOException e) { + throw new Error(e); + } + } + + static private ModelsEvaluator createModels(String path) { + Path configDir = Path.fromString(path); + RankProfilesConfig config = new ConfigGetter<>(new FileSource(configDir.append("rank-profiles.cfg").toFile()), + RankProfilesConfig.class).getConfig(""); + RankingConstantsConfig constantsConfig = new ConfigGetter<>(new FileSource(configDir.append("ranking-constants.cfg").toFile()), + RankingConstantsConfig.class).getConfig(""); + ModelTester.RankProfilesConfigImporterWithMockedConstants importer = + new ModelTester.RankProfilesConfigImporterWithMockedConstants(Path.fromString(path).append("constants"), + MockFileAcquirer.returnFile(null)); + return new ModelsEvaluator(importer.importFrom(config, constantsConfig)); + } + +} diff --git a/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.cpp index 7e835373e41..7b1ed5d6522 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.cpp +++ b/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.cpp @@ -4,13 +4,27 @@ namespace proton { +ContentProtonMetrics::ProtonExecutorMetrics::ProtonExecutorMetrics(metrics::MetricSet *parent) + : metrics::MetricSet("executor", "", "Metrics for top-level executors shared among all document databases", parent), + proton("proton", this), + flush("flush", this), + match("match", this), + docsum("docsum", this), + shared("shared", this), + warmup("warmup", this) +{ +} + +ContentProtonMetrics::ProtonExecutorMetrics::~ProtonExecutorMetrics() = default; + ContentProtonMetrics::ContentProtonMetrics() : metrics::MetricSet("content.proton", "", "Search engine metrics", nullptr), transactionLog(this), - resourceUsage(this) + resourceUsage(this), + executor(this) { } -ContentProtonMetrics::~ContentProtonMetrics() {} +ContentProtonMetrics::~ContentProtonMetrics() = default; -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h index d2a53b12a02..ef66314f658 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h @@ -2,9 +2,10 @@ #pragma once -#include <vespa/metrics/metrics.h> +#include "executor_metrics.h" #include "resource_usage_metrics.h" #include "trans_log_server_metrics.h" +#include <vespa/metrics/metrics.h> namespace proton { @@ -18,12 +19,26 @@ namespace proton { */ struct ContentProtonMetrics : metrics::MetricSet { + struct ProtonExecutorMetrics : metrics::MetricSet { + + ExecutorMetrics proton; + ExecutorMetrics flush; + ExecutorMetrics match; + ExecutorMetrics docsum; + ExecutorMetrics shared; + ExecutorMetrics warmup; + + ProtonExecutorMetrics(metrics::MetricSet *parent); + ~ProtonExecutorMetrics(); + }; + TransLogServerMetrics transactionLog; ResourceUsageMetrics resourceUsage; + ProtonExecutorMetrics executor; ContentProtonMetrics(); ~ContentProtonMetrics(); }; -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.cpp index 4d23c3f1603..3b24dcdc1d1 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.cpp +++ b/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.cpp @@ -25,7 +25,6 @@ LegacyDocumentDBMetrics::DocstoreMetrics::DocstoreMetrics(MetricSet *parent) : MetricSet("docstore", "", "Document store metrics", parent), memoryUsage("memoryusage", "", "Memory usage for docstore", this), cacheLookups("cachelookups", "", "Number of lookups in summary cache", this), - hits(0), cacheHitRate("cachehitrate", "", "Rate of cache hits in summary cache", this), cacheElements("cacheelements", "", "Number of elements in summary cache", this), cacheMemoryUsed("cachememoryused", "", "Memory used by summary cache", this) diff --git a/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.h index 2c66428aabd..7b01842103b 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/legacy_documentdb_metrics.h @@ -32,7 +32,6 @@ struct LegacyDocumentDBMetrics : metrics::MetricSet struct DocstoreMetrics : metrics::MetricSet { metrics::LongValueMetric memoryUsage; metrics::LongCountMetric cacheLookups; - size_t hits; metrics::LongAverageMetric cacheHitRate; metrics::LongValueMetric cacheElements; metrics::LongValueMetric cacheMemoryUsed; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index ec6db6fa5b8..c974f812acc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -83,7 +83,7 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, const ProtonConfig &protonCfg, IDocumentDBOwner &owner, vespalib::ThreadExecutor &warmupExecutor, - vespalib::ThreadStackExecutorBase &summaryExecutor, + vespalib::ThreadStackExecutorBase &sharedExecutor, search::transactionlog::Writer &tlsDirectWriter, MetricsWireService &metricsWireService, const FileHeaderContext &fileHeaderContext, @@ -130,9 +130,9 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, _writeFilter(), _feedHandler(_writeService, tlsSpec, docTypeName, _state, *this, _writeFilter, *this, tlsDirectWriter), _subDBs(*this, *this, _feedHandler, _docTypeName, _writeService, warmupExecutor, - summaryExecutor, fileHeaderContext, metricsWireService, getMetricsCollection(), + sharedExecutor, fileHeaderContext, metricsWireService, getMetricsCollection(), queryLimiter, clock, _configMutex, _baseDir, protonCfg, hwInfo), - _maintenanceController(_writeService.master(), summaryExecutor, _docTypeName), + _maintenanceController(_writeService.master(), sharedExecutor, _docTypeName), _visibility(_feedHandler, _writeService, _feedView), _lidSpaceCompactionHandlers(), _jobTrackers(), @@ -1209,7 +1209,6 @@ updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics, metrics.memoryUsage.set(memoryUsage); updateCountMetric(cache_stats.lookups(), lastCacheStats.lookups(), metrics.cacheLookups); updateDocumentStoreCacheHitRate(cache_stats, lastCacheStats, metrics.cacheHitRate); - metrics.hits = cache_stats.hits; metrics.cacheElements.set(cache_stats.elements); metrics.cacheMemoryUsed.set(cache_stats.memory_used); lastCacheStats = cache_stats; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.h b/searchcore/src/vespa/searchcore/proton/server/documentdb.h index 5c04c4057ae..996b365cb48 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.h @@ -250,7 +250,7 @@ public: const ProtonConfig &protonCfg, IDocumentDBOwner &owner, vespalib::ThreadExecutor &warmupExecutor, - vespalib::ThreadStackExecutorBase &summaryExecutor, + vespalib::ThreadStackExecutorBase &sharedExecutor, search::transactionlog::Writer &tlsDirectWriter, MetricsWireService &metricsWireService, const search::common::FileHeaderContext &fileHeaderContext, diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp index 001b0573e0b..b67d09480ec 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp @@ -37,7 +37,7 @@ DocumentSubDBCollection::DocumentSubDBCollection( const DocTypeName &docTypeName, searchcorespi::index::IThreadingService &writeService, vespalib::ThreadExecutor &warmupExecutor, - vespalib::ThreadStackExecutorBase &summaryExecutor, + vespalib::ThreadStackExecutorBase &sharedExecutor, const search::common::FileHeaderContext &fileHeaderContext, MetricsWireService &metricsWireService, DocumentDBMetricsCollection &metrics, @@ -73,7 +73,7 @@ DocumentSubDBCollection::DocumentSubDBCollection( getSerialNum, fileHeaderContext, writeService, - summaryExecutor, + sharedExecutor, _bucketDB, *_bucketDBHandler, metrics, diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h index ea07e391f69..ebc418497dc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h @@ -82,7 +82,7 @@ public: const DocTypeName &docTypeName, searchcorespi::index::IThreadingService &writeService, vespalib::ThreadExecutor &warmupExecutor, - vespalib::ThreadStackExecutorBase &summaryExecutor, + vespalib::ThreadStackExecutorBase &sharedExecutor, const search::common::FileHeaderContext &fileHeaderContext, MetricsWireService &metricsWireService, DocumentDBMetricsCollection &metrics, diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 070a3172fe0..2174412bedf 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -194,7 +194,7 @@ Proton::Proton(const config::ConfigUri & configUri, _protonConfigurer(_executor, *this, _protonDiskLayout), _protonConfigFetcher(configUri, _protonConfigurer, subscribeTimeout), _warmupExecutor(), - _summaryExecutor(), + _sharedExecutor(), _queryLimiter(), _clock(0.010), _threadPool(128 * 1024), @@ -288,8 +288,8 @@ Proton::init(const BootstrapConfig::SP & configSnapshot) vespalib::string fileConfigId; _warmupExecutor = std::make_unique<vespalib::ThreadStackExecutor>(4, 128*1024); - const size_t summaryThreads = deriveCompactionCompressionThreads(protonConfig, hwInfo.cpu()); - _summaryExecutor = std::make_unique<vespalib::BlockingThreadStackExecutor>(summaryThreads, 128*1024, summaryThreads*16); + const size_t sharedThreads = deriveCompactionCompressionThreads(protonConfig, hwInfo.cpu()); + _sharedExecutor = std::make_unique<vespalib::BlockingThreadStackExecutor>(sharedThreads, 128*1024, sharedThreads*16); InitializeThreads initializeThreads; if (protonConfig.initialize.threads > 0) { initializeThreads = std::make_shared<vespalib::ThreadStackExecutor>(protonConfig.initialize.threads, 128 * 1024); @@ -422,8 +422,8 @@ Proton::~Proton() if (_warmupExecutor) { _warmupExecutor->sync(); } - if (_summaryExecutor) { - _summaryExecutor->sync(); + if (_sharedExecutor) { + _sharedExecutor->sync(); } LOG(debug, "Shutting down fs4 interface"); if (_metricsEngine && _fs4Server) { @@ -440,7 +440,7 @@ Proton::~Proton() _persistenceEngine.reset(); _tls.reset(); _warmupExecutor.reset(); - _summaryExecutor.reset(); + _sharedExecutor.reset(); _clock.stop(); LOG(debug, "Explicit destructor done"); } @@ -526,7 +526,7 @@ Proton::addDocumentDB(const document::DocumentType &docType, } auto ret = std::make_shared<DocumentDB>(config.basedir + "/documents", documentDBConfig, config.tlsspec, _queryLimiter, _clock, docTypeName, bucketSpace, config, *this, - *_warmupExecutor, *_summaryExecutor, *_tls->getTransLogServer(), + *_warmupExecutor, *_sharedExecutor, *_tls->getTransLogServer(), *_metricsEngine, _fileHeaderContext, std::move(config_store), initializeThreads, bootstrapConfig->getHwInfo()); try { @@ -658,7 +658,15 @@ int countOpenFiles() return count; } -} // namespace <unnamed> +void +updateExecutorMetrics(ExecutorMetrics &metrics, ExecutorMetrics &legacyMetrics, + const vespalib::ThreadStackExecutor::Stats &stats) +{ + metrics.update(stats); + legacyMetrics.update(stats); +} + +} void Proton::updateMetrics(const vespalib::MonitorGuard &) @@ -681,16 +689,23 @@ Proton::updateMetrics(const vespalib::MonitorGuard &) metrics.resourceUsage.feedingBlocked.set((usageFilter.acceptWriteOperation() ? 0.0 : 1.0)); } { - LegacyProtonMetrics &metrics = _metricsEngine->legacyRoot(); - metrics.executor.update(_executor.getStats()); + ContentProtonMetrics::ProtonExecutorMetrics &metrics = _metricsEngine->root().executor; + LegacyProtonMetrics &legacyMetrics = _metricsEngine->legacyRoot(); + updateExecutorMetrics(metrics.proton, legacyMetrics.executor, _executor.getStats()); if (_flushEngine) { - metrics.flushExecutor.update(_flushEngine->getExecutorStats()); + updateExecutorMetrics(metrics.flush, legacyMetrics.flushExecutor, _flushEngine->getExecutorStats()); } if (_matchEngine) { - metrics.matchExecutor.update(_matchEngine->getExecutorStats()); + updateExecutorMetrics(metrics.match, legacyMetrics.matchExecutor, _matchEngine->getExecutorStats()); } if (_summaryEngine) { - metrics.summaryExecutor.update(_summaryEngine->getExecutorStats()); + updateExecutorMetrics(metrics.docsum, legacyMetrics.summaryExecutor, _summaryEngine->getExecutorStats()); + } + if (_sharedExecutor) { + metrics.shared.update(_sharedExecutor->getStats()); + } + if (_warmupExecutor) { + metrics.warmup.update(_warmupExecutor->getStats()); } } } diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.h b/searchcore/src/vespa/searchcore/proton/server/proton.h index 6e07ddcecdb..7dd4630360a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.h +++ b/searchcore/src/vespa/searchcore/proton/server/proton.h @@ -116,7 +116,7 @@ private: ProtonConfigurer _protonConfigurer; ProtonConfigFetcher _protonConfigFetcher; std::unique_ptr<vespalib::ThreadStackExecutorBase> _warmupExecutor; - std::unique_ptr<vespalib::ThreadStackExecutorBase> _summaryExecutor; + std::unique_ptr<vespalib::ThreadStackExecutorBase> _sharedExecutor; matching::QueryLimiter _queryLimiter; vespalib::Clock _clock; FastOS_ThreadPool _threadPool; diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp index acdda10a7ef..02afaddd830 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp @@ -81,7 +81,7 @@ StoreOnlyDocSubDB::Context::Context(IDocumentSubDBOwner &owner, const IGetSerialNum &getSerialNum, const search::common::FileHeaderContext &fileHeaderContext, searchcorespi::index::IThreadingService &writeService, - vespalib::ThreadStackExecutorBase &summaryExecutor, + vespalib::ThreadStackExecutorBase &sharedExecutor, std::shared_ptr<BucketDBOwner> bucketDB, bucketdb::IBucketDBHandlerInitializer & bucketDBHandlerInitializer, DocumentDBMetricsCollection &metrics, @@ -92,7 +92,7 @@ StoreOnlyDocSubDB::Context::Context(IDocumentSubDBOwner &owner, _getSerialNum(getSerialNum), _fileHeaderContext(fileHeaderContext), _writeService(writeService), - _summaryExecutor(summaryExecutor), + _sharedExecutor(sharedExecutor), _bucketDB(bucketDB), _bucketDBHandlerInitializer(bucketDBHandlerInitializer), _metrics(metrics), @@ -118,7 +118,7 @@ StoreOnlyDocSubDB::StoreOnlyDocSubDB(const Config &cfg, const Context &ctx) _rSummaryMgr(), _summaryAdapter(), _writeService(ctx._writeService), - _summaryExecutor(ctx._summaryExecutor), + _sharedExecutor(ctx._sharedExecutor), _metrics(ctx._metrics), _iSearchView(), _iFeedView(), @@ -238,7 +238,7 @@ createSummaryManagerInitializer(const search::LogDocumentStore::Config & storeCf GrowStrategy grow = _attributeGrow; vespalib::string baseDir(_baseDir + "/summary"); return std::make_shared<SummaryManagerInitializer> - (grow, baseDir, getSubDbName(), _docTypeName, _summaryExecutor, + (grow, baseDir, getSubDbName(), _docTypeName, _sharedExecutor, storeCfg, tuneFile, _fileHeaderContext, _tlSyncer, bucketizer, result); } diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h index 9b30ce4d65e..4a21537f134 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h @@ -110,7 +110,7 @@ public: const IGetSerialNum &_getSerialNum; const search::common::FileHeaderContext &_fileHeaderContext; searchcorespi::index::IThreadingService &_writeService; - vespalib::ThreadStackExecutorBase &_summaryExecutor; + vespalib::ThreadStackExecutorBase &_sharedExecutor; std::shared_ptr<BucketDBOwner> _bucketDB; bucketdb::IBucketDBHandlerInitializer &_bucketDBHandlerInitializer; DocumentDBMetricsCollection &_metrics; @@ -122,7 +122,7 @@ public: const IGetSerialNum &getSerialNum, const search::common::FileHeaderContext &fileHeaderContext, searchcorespi::index::IThreadingService &writeService, - vespalib::ThreadStackExecutorBase &summaryExecutor, + vespalib::ThreadStackExecutorBase &sharedExecutor, std::shared_ptr<BucketDBOwner> bucketDB, bucketdb::IBucketDBHandlerInitializer & bucketDBHandlerInitializer, @@ -153,7 +153,7 @@ private: ISummaryAdapter::SP _summaryAdapter; protected: searchcorespi::index::IThreadingService &_writeService; - vespalib::ThreadStackExecutorBase &_summaryExecutor; + vespalib::ThreadStackExecutorBase &_sharedExecutor; DocumentDBMetricsCollection &_metrics; vespalib::VarHolder<ISearchHandler::SP> _iSearchView; vespalib::VarHolder<IFeedView::SP> _iFeedView; diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp index d0886c25898..fbf752d3893 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp @@ -138,12 +138,11 @@ void AttributeVector::updateStat(bool force) { onUpdateStat(); } else if (_nextStatUpdateTime < fastos::ClockSystem::now()) { onUpdateStat(); - _nextStatUpdateTime = fastos::ClockSystem::now() + fastos::TimeStamp::SEC; + _nextStatUpdateTime = fastos::ClockSystem::now() + 5ul * fastos::TimeStamp::SEC; } } bool AttributeVector::hasEnum() const { return _hasEnum; } -bool AttributeVector::hasEnum2Value() const { return false; } uint32_t AttributeVector::getMaxValueCount() const { return _highestValueCount; } bool diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h index 54a43bec09e..ab590f807bf 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.h +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h @@ -42,7 +42,6 @@ namespace search { template <typename T> class ComponentGuard; class AttributeReadGuard; - class AttributeWriteGuard; class AttributeSaver; class EnumStoreBase; class IAttributeSaveTarget; @@ -405,7 +404,6 @@ public: bool hasArrayType() const { return _config.collectionType().isArray(); } bool hasEnum() const override final; - virtual bool hasEnum2Value() const; uint32_t getMaxValueCount() const override; uint32_t getEnumMax() const { return _enumMax; } diff --git a/searchlib/src/vespa/searchlib/attribute/attrvector.h b/searchlib/src/vespa/searchlib/attribute/attrvector.h index c0530ee8368..eb6d2eebb84 100644 --- a/searchlib/src/vespa/searchlib/attribute/attrvector.h +++ b/searchlib/src/vespa/searchlib/attribute/attrvector.h @@ -42,7 +42,7 @@ protected: typedef typename B::Config Config; NumericDirectAttribute(const vespalib::string & baseFileName, const Config & c); - ~NumericDirectAttribute(); + ~NumericDirectAttribute() override; bool findEnum(BaseType value, EnumHandle & e) const override; void onCommit() override; @@ -83,7 +83,6 @@ private: uint32_t get(DocId doc, EnumHandle * e, uint32_t sz) const override { return getAllEnumHelper(doc, e, sz); } uint32_t getValueCount(DocId doc) const override { return getValueCountHelper(doc); } - bool hasEnum2Value() const override { return false; } uint32_t getValueCountHelper(DocId doc) const { if (F::IsMultiValue()) { @@ -145,7 +144,7 @@ private: const char * getStringFromEnum(EnumHandle e) const override { return &_buffer[e]; } protected: StringDirectAttribute(const vespalib::string & baseFileName, const Config & c); - ~StringDirectAttribute(); + ~StringDirectAttribute() override; bool findEnum(const char * value, EnumHandle & e) const override; std::vector<EnumHandle> findFoldedEnums(const char *) const override; void onCommit() override; @@ -182,7 +181,6 @@ private: uint32_t get(DocId doc, WeightedEnum * e, uint32_t sz) const override { return getAllEnumHelper(doc, e, sz); } uint32_t get(DocId doc, WeightedString * v, uint32_t sz) const override { return getAllHelper(doc, v, sz); } uint32_t get(DocId doc, WeightedConstChar * v, uint32_t sz) const override { return getAllHelper(doc, v, sz); } - bool hasEnum2Value() const override { return true; } uint32_t getValueCountHelper(DocId doc) const { if (F::IsMultiValue()) { diff --git a/searchlib/src/vespa/searchlib/attribute/attrvector.hpp b/searchlib/src/vespa/searchlib/attribute/attrvector.hpp index 565801b1b0c..cdd34725e69 100644 --- a/searchlib/src/vespa/searchlib/attribute/attrvector.hpp +++ b/searchlib/src/vespa/searchlib/attribute/attrvector.hpp @@ -18,7 +18,7 @@ NumericDirectAttribute(const vespalib::string & baseFileName, const Config & c) } template <typename B> -NumericDirectAttribute<B>::~NumericDirectAttribute() {} +NumericDirectAttribute<B>::~NumericDirectAttribute() = default; template <typename B> bool NumericDirectAttribute<B>::onLoad() diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.h b/searchlib/src/vespa/searchlib/attribute/enumattribute.h index 993267f79a6..c79c9a7c2fb 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.h @@ -73,9 +73,7 @@ protected: void insertNewUniqueValues(EnumStoreBase::IndexVector & newIndexes); virtual void considerAttributeChange(const Change & c, UniqueSet & newUniques) = 0; virtual void reEnumerate() = 0; - bool hasEnum2Value() const override { return true; } AddressSpace getEnumStoreAddressSpaceUsage() const override; - public: EnumAttribute(const vespalib::string & baseFileName, const AttributeVector::Config & cfg); ~EnumAttribute(); diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h index 435fbb21923..dd88393a5e2 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h @@ -6,8 +6,7 @@ #include <vespa/searchlib/datastore/array_store.h> #include <vespa/searchlib/common/address_space.h> -namespace search { -namespace attribute { +namespace search::attribute { /** * Class for mapping from from document id to an array of values. @@ -29,7 +28,7 @@ public: MultiValueMapping & operator = (const MultiValueMapping &) = delete; MultiValueMapping(const datastore::ArrayStoreConfig &storeCfg, const GrowStrategy &gs = GrowStrategy()); - virtual ~MultiValueMapping(); + ~MultiValueMapping() override; ConstArrayRef get(uint32_t docId) const { return _store.get(_indices[docId]); } ConstArrayRef getDataForIdx(EntryRef idx) const { return _store.get(idx); } void set(uint32_t docId, ConstArrayRef values); @@ -45,10 +44,10 @@ public: void doneLoadFromMultiValue() { _store.setInitializing(false); } - virtual void compactWorst(bool compactMemory, bool compactAddressSpace) override; + void compactWorst(bool compactMemory, bool compactAddressSpace) override; - virtual AddressSpace getAddressSpaceUsage() const override; - virtual MemoryUsage getArrayStoreMemoryUsage() const override; + AddressSpace getAddressSpaceUsage() const override; + MemoryUsage getArrayStoreMemoryUsage() const override; static datastore::ArrayStoreConfig optimizedConfigForHugePage(size_t maxSmallArraySize, size_t hugePageSize, @@ -57,5 +56,4 @@ public: float allocGrowFactor); }; -} // namespace search::attribute -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp index 83886619d0f..2395cb8b808 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp @@ -6,8 +6,7 @@ #include <vespa/searchlib/datastore/array_store.hpp> #include <vespa/searchlib/common/rcuvector.hpp> -namespace search { -namespace attribute { +namespace search::attribute { template <typename EntryT, typename RefT> MultiValueMapping<EntryT,RefT>::MultiValueMapping(const datastore::ArrayStoreConfig &storeCfg, const GrowStrategy &gs) @@ -17,9 +16,7 @@ MultiValueMapping<EntryT,RefT>::MultiValueMapping(const datastore::ArrayStoreCon } template <typename EntryT, typename RefT> -MultiValueMapping<EntryT,RefT>::~MultiValueMapping() -{ -} +MultiValueMapping<EntryT,RefT>::~MultiValueMapping() = default; template <typename EntryT, typename RefT> void @@ -52,8 +49,7 @@ MultiValueMapping<EntryT,RefT>::compactWorst(bool compactMemory, bool compactAdd { datastore::ICompactionContext::UP compactionContext(_store.compactWorst(compactMemory, compactAddressSpace)); if (compactionContext) { - compactionContext->compact(vespalib::ArrayRef<EntryRef>(&_indices[0], - _indices.size())); + compactionContext->compact(vespalib::ArrayRef<EntryRef>(&_indices[0], _indices.size())); } } @@ -81,5 +77,4 @@ MultiValueMapping<EntryT, RefT>::optimizedConfigForHugePage(size_t maxSmallArray return ArrayStore::optimizedConfigForHugePage(maxSmallArraySize, hugePageSize, smallPageSize, minNumArraysForNewBuffer, allocGrowFactor); } -} // namespace search::attribute -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp index c3046b5ed7c..67257286a70 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp @@ -3,8 +3,7 @@ #include "multi_value_mapping_base.h" #include <vespa/searchcommon/common/compaction_strategy.h> -namespace search { -namespace attribute { +namespace search::attribute { namespace { @@ -23,9 +22,7 @@ MultiValueMappingBase::MultiValueMappingBase(const GrowStrategy &gs, { } -MultiValueMappingBase::~MultiValueMappingBase() -{ -} +MultiValueMappingBase::~MultiValueMappingBase() = default; MultiValueMappingBase::RefCopyVector MultiValueMappingBase::getRefCopy(uint32_t size) const { @@ -102,5 +99,4 @@ MultiValueMappingBase::considerCompact(const CompactionStrategy &compactionStrat return false; } -} // namespace search::attribute -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h index 5affd893d4d..faf16a8f624 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h @@ -7,11 +7,9 @@ #include <vespa/searchlib/common/address_space.h> #include <functional> -namespace search { +namespace search { class CompactionStrategy; } -class CompactionStrategy; - -namespace attribute { +namespace search::attribute { /** * Base class for mapping from from document id to an array of values. @@ -57,5 +55,4 @@ public: bool considerCompact(const CompactionStrategy &compactionStrategy); }; -} // namespace search::attribute -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.h b/searchlib/src/vespa/searchlib/attribute/multienumattribute.h index bcad27f046e..99f8d594976 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.h @@ -58,7 +58,7 @@ protected: this->getEnumStore().freezeTree(); } - virtual void fillValues(LoadedVector & loaded) override; + void fillValues(LoadedVector & loaded) override; void fillEnumIdx(ReaderBase &attrReader, const EnumIndexVector &eidxs, LoadedEnumAttributeVector &loaded) override; void fillEnumIdx(ReaderBase &attrReader, const EnumIndexVector &eidxs, EnumVector &enumHist) override; virtual void mergeMemoryStats(MemoryUsage & total) { (void) total; } diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h index 4957e4b68cb..7ccde24aff3 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h @@ -53,7 +53,7 @@ protected: public: MultiValueAttribute(const vespalib::string & baseFileName, const AttributeVector::Config & cfg); - virtual ~MultiValueAttribute(); + ~MultiValueAttribute() override; bool addDoc(DocId & doc) override; uint32_t getValueCount(DocId doc) const override; diff --git a/searchlib/src/vespa/searchlib/datastore/array_store.hpp b/searchlib/src/vespa/searchlib/datastore/array_store.hpp index d76bd173d4a..14c6a43fe09 100644 --- a/searchlib/src/vespa/searchlib/datastore/array_store.hpp +++ b/searchlib/src/vespa/searchlib/datastore/array_store.hpp @@ -139,10 +139,10 @@ public: _store(store), _bufferIdsToCompact(std::move(bufferIdsToCompact)) {} - virtual ~CompactionContext() { + ~CompactionContext() override { _dataStore.finishCompact(_bufferIdsToCompact); } - virtual void compact(vespalib::ArrayRef<EntryRef> refs) override { + void compact(vespalib::ArrayRef<EntryRef> refs) override { if (!_bufferIdsToCompact.empty()) { for (auto &ref : refs) { if (ref.valid()) { @@ -202,4 +202,3 @@ ArrayStore<EntryT, RefT>::optimizedConfigForHugePage(size_t maxSmallArraySize, } } - diff --git a/searchlib/src/vespa/searchlib/datastore/datastorebase.cpp b/searchlib/src/vespa/searchlib/datastore/datastorebase.cpp index 68e7155505f..29621e79a59 100644 --- a/searchlib/src/vespa/searchlib/datastore/datastorebase.cpp +++ b/searchlib/src/vespa/searchlib/datastore/datastorebase.cpp @@ -61,17 +61,14 @@ class DataStoreBase::BufferHold : public GenerationHeldBase uint32_t _bufferId; public: - BufferHold(size_t size, - DataStoreBase &dsb, - uint32_t bufferId) + BufferHold(size_t size, DataStoreBase &dsb, uint32_t bufferId) : GenerationHeldBase(size), _dsb(dsb), _bufferId(bufferId) { } - virtual - ~BufferHold() + ~BufferHold() override { _dsb.doneHoldBuffer(_bufferId); } diff --git a/searchlib/src/vespa/searchlib/datastore/datastorebase.h b/searchlib/src/vespa/searchlib/datastore/datastorebase.h index bdb49fec029..b5256545194 100644 --- a/searchlib/src/vespa/searchlib/datastore/datastorebase.h +++ b/searchlib/src/vespa/searchlib/datastore/datastorebase.h @@ -13,10 +13,6 @@ namespace search::datastore { class DataStoreBase { -private: - DataStoreBase(const DataStoreBase &rhs); - - DataStoreBase &operator=(const DataStoreBase &rhs); public: // Hold list before freeze, before knowing how long elements must be held class ElemHold1ListElem @@ -80,7 +76,7 @@ protected: FallbackHold(size_t size, BufferState::Alloc &&buffer, size_t usedElems, BufferTypeBase *typeHandler, uint32_t typeId); - virtual ~FallbackHold(); + ~FallbackHold() override; }; class BufferHold; @@ -151,6 +147,8 @@ protected: vespalib::GenerationHolder _genHolder; DataStoreBase(uint32_t numBuffers, size_t maxClusters); + DataStoreBase(const DataStoreBase &) = delete; + DataStoreBase &operator=(const DataStoreBase &) = delete; virtual ~DataStoreBase(); diff --git a/vespajlib/src/main/java/com/yahoo/collections/ConcurrentResourcePool.java b/vespajlib/src/main/java/com/yahoo/collections/ConcurrentResourcePool.java deleted file mode 100644 index b49e0c8bbbc..00000000000 --- a/vespajlib/src/main/java/com/yahoo/collections/ConcurrentResourcePool.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.collections; - -import java.util.Iterator; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; - -/** - * @author baldersheim - * TODO: remove on vespa 7 or before - * Use com.yahoo.yolean.concurrent.ConcurrentResourcePool instead. - */ -@Deprecated -public class ConcurrentResourcePool<T> implements Iterable<T> { - - private final Queue<T> pool = new ConcurrentLinkedQueue<>(); - private final ResourceFactory<T> factory; - - public ConcurrentResourcePool(ResourceFactory<T> factory) { - this.factory = factory; - } - - public final T alloc() { - final T e = pool.poll(); - return e != null ? e : factory.create(); - } - - public final void free(T e) { - pool.offer(e); - } - - @Override - public Iterator<T> iterator() { - return pool.iterator(); - } -} diff --git a/vespajlib/src/main/java/com/yahoo/collections/ResourceFactory.java b/vespajlib/src/main/java/com/yahoo/collections/ResourceFactory.java deleted file mode 100644 index 7a8e7dafbc4..00000000000 --- a/vespajlib/src/main/java/com/yahoo/collections/ResourceFactory.java +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.collections; - -/** - * @author baldersheim - * TODO: remove on vespa 7 or before - * Use com.yahoo.yolean.concurrent.ResourceFactory instead. - */ -@Deprecated -public abstract class ResourceFactory<T> { - - public abstract T create(); -} diff --git a/vespajlib/src/main/java/com/yahoo/collections/ResourcePool.java b/vespajlib/src/main/java/com/yahoo/collections/ResourcePool.java deleted file mode 100644 index a6ad8b96a04..00000000000 --- a/vespajlib/src/main/java/com/yahoo/collections/ResourcePool.java +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.collections; - -import java.util.ArrayDeque; -import java.util.Deque; -import java.util.Iterator; - -/** - * <p>This implements a simple stack based resource pool. If you are out of resources new are allocated from the - * factory.</p> - * - * @author baldersheim - * TODO: remove on vespa 7 or before - * Use com.yahoo.yolean.concurrent.ResourceFactory instead. - */ -@Deprecated -public final class ResourcePool<T> implements Iterable<T> { - - private final Deque<T> pool = new ArrayDeque<>(); - private final ResourceFactory<T> factory; - - public ResourcePool(ResourceFactory<T> factory) { - this.factory = factory; - } - - public final T alloc() { - return pool.isEmpty() ? factory.create() : pool.pop(); - } - - public final void free(T e) { - pool.push(e); - } - - @Override - public Iterator<T> iterator() { - return pool.iterator(); - } -} |