diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/chains/search/DomSearchChainBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/chains/search/DomSearchChainBuilder.java index 4ada9e99e5e5..828bfce0f94c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/chains/search/DomSearchChainBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/chains/search/DomSearchChainBuilder.java @@ -16,6 +16,7 @@ /** * Builds a Search chain from xml. + * * @author Tony Vaagenes */ public class DomSearchChainBuilder extends DomChainBuilderBase, SearchChain> { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java index 0a8ae69ccf0d..e8fc39eb8d5d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java @@ -22,7 +22,7 @@ public FederationOptions federationOptions() { return new FederationOptions().setUseByDefault(true); } - //A list of documents types that this search chain provides results for, empty if unknown + /** A list of documents types that this search chain provides results for, empty if unknown. */ public List getDocumentTypes() { return List.of(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java index aaf1824e0c78..638654bcd0be 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java @@ -30,33 +30,10 @@ // TODO: refactor public class VespaSearchChainsCreator { - private static class PhasesCreator { - - private static Set set(String successor) { - return successor == null ? null : new LinkedHashSet<>(List.of(successor)); - } - - private static String lastElement(String[] phases) { - return phases[phases.length - 1]; - } - - private static Phase createPhase(String phase, String before) { - return new Phase(phase, set(before), null); - } - - static Collection linearPhases(String... phases) { - List result = new ArrayList<>(); - - for (int i=0; i < phases.length - 1; ++i) { - result.add(createPhase(phases[i], phases[i+1])); - } - - if (phases.length > 0) { - result.add(createPhase(lastElement(phases), null)); - } - - return result; - } + public static void addVespaSearchChains(SearchChains searchChains) { + searchChains.add(createVespaPhases()); + searchChains.add(createNative()); + searchChains.add(createVespa()); } private static Set noSearcherReferences() { @@ -133,10 +110,33 @@ private static SearchChain createVespa() { return vespaChain; } - public static void addVespaSearchChains(SearchChains searchChains) { - searchChains.add(createVespaPhases()); - searchChains.add(createNative()); - searchChains.add(createVespa()); + private static class PhasesCreator { + + private static Set set(String successor) { + return successor == null ? null : new LinkedHashSet<>(List.of(successor)); + } + + private static String lastElement(String[] phases) { + return phases[phases.length - 1]; + } + + private static Phase createPhase(String phase, String before) { + return new Phase(phase, set(before), null); + } + + static Collection linearPhases(String... phases) { + List result = new ArrayList<>(); + + for (int i=0; i < phases.length - 1; ++i) { + result.add(createPhase(phases[i], phases[i+1])); + } + + if (phases.length > 0) { + result.add(createPhase(lastElement(phases), null)); + } + + return result; + } } } diff --git a/container-disc/src/main/java/com/yahoo/component/chain/Chain.java b/container-disc/src/main/java/com/yahoo/component/chain/Chain.java index 6f69bdbdfe89..bbdb1871db5e 100644 --- a/container-disc/src/main/java/com/yahoo/component/chain/Chain.java +++ b/container-disc/src/main/java/com/yahoo/component/chain/Chain.java @@ -56,10 +56,7 @@ public Chain(ComponentId id, COMPONENT... components) { /** Create a chain by using a builder. This will order the chain by the ordering constraints. */ public Chain(ComponentId id, Collection components, Collection phases) { - this(id, buildChain( - emptyListIfNull(components), - emptyListIfNull(phases)).components()); - + this(id, buildChain(emptyListIfNull(components), emptyListIfNull(phases)).components()); } public ComponentId getId() { diff --git a/container-disc/src/main/java/com/yahoo/component/chain/ChainsConfigurer.java b/container-disc/src/main/java/com/yahoo/component/chain/ChainsConfigurer.java index 937e33e95635..9859dc47a4eb 100644 --- a/container-disc/src/main/java/com/yahoo/component/chain/ChainsConfigurer.java +++ b/container-disc/src/main/java/com/yahoo/component/chain/ChainsConfigurer.java @@ -39,10 +39,8 @@ private static void initDependencies( } } - private static COMPONENT getComponentOrThrow( - ComponentRegistry registry, - ComponentSpecification specification) { - + private static COMPONENT getComponentOrThrow(ComponentRegistry registry, + ComponentSpecification specification) { COMPONENT component = registry.getComponent(specification); if (component == null) { throw new ConfigurationRuntimeException("No such component '" + specification + "'"); @@ -59,8 +57,8 @@ private static void instantiateChains( for (ChainSpecification chain : model.allChainsFlattened()) { try { Chain componentChain = new Chain<>(chain.componentId, - resolveComponents(chain.componentReferences, allComponents), - chain.phases()); + resolveComponents(chain.componentReferences, allComponents), + chain.phases()); chainRegistry.register(chain.componentId, componentChain); } catch (Exception e) { throw new ConfigurationRuntimeException("Invalid chain '" + chain.componentId + "'", e); @@ -68,9 +66,8 @@ private static void instantiateChains( } } - private static List resolveComponents( - Set componentSpecifications, - ComponentRegistry allComponents) { + private static List resolveComponents(Set componentSpecifications, + ComponentRegistry allComponents) { List components = new ArrayList<>(componentSpecifications.size()); for (ComponentSpecification componentSpec : componentSpecifications) { diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java index 2cb6dc9a5892..0d7e4da1e063 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java @@ -40,8 +40,6 @@ public abstract class ThreadedHttpRequestHandler extends ThreadedRequestHandler /** Logger for subclasses */ protected final Logger log; - - public ThreadedHttpRequestHandler(Executor executor) { this(executor, null); } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java index 42a203576e27..ce9b73bd09c3 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java @@ -202,7 +202,7 @@ private void processRequest() { @Override public ContentChannel handleResponse(Response response) { - if ( tryHasResponded()) throw new IllegalStateException("Response already handled"); + if (tryHasResponded()) throw new IllegalStateException("Response already handled"); if (getRequestType().isPresent() && response.getRequestType() == null) response.setRequestType(getRequestType().get()); ContentChannel cc = responseHandler.handleResponse(response); diff --git a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java index b358800e03d1..a002bca388db 100644 --- a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java +++ b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java @@ -9,9 +9,11 @@ /** * Responsible for metric reporting for JDisc http request handler support. + * * @author Tony Vaagenes */ class RequestMetricReporter { + private final Metric metric; private final Context context; @@ -48,17 +50,13 @@ void failedWrite() { void successfulResponse() { setTimeToFirstByteFirstTime(); - long requestLatency = getRequestLatency(); - metric.set(MetricDefinitions.TOTAL_SUCCESSFUL_LATENCY, requestLatency, context); - metric.add(MetricDefinitions.NUM_SUCCESSFUL_RESPONSES, 1, context); } void failedResponse() { setTimeToFirstByteFirstTime(); - metric.set(MetricDefinitions.TOTAL_FAILED_LATENCY, getRequestLatency(), context); metric.add(MetricDefinitions.NUM_FAILED_RESPONSES, 1, context); } @@ -82,4 +80,5 @@ void uriLength(int length) { void contentSize(long size) { metric.set(MetricDefinitions.CONTENT_SIZE, size, context); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java index 39325c77d402..246de3fe926e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java @@ -17,7 +17,6 @@ import com.yahoo.search.result.Hit; import com.yahoo.search.result.HitGroup; import com.yahoo.search.searchchain.Execution; -import com.yahoo.search.searchchain.PhaseNames; import java.util.HashMap; import java.util.Map; @@ -41,18 +40,16 @@ /** - *

A searcher to gather statistics such as queries completed and query latency. There + * A searcher to gather statistics such as queries completed and query latency. There * may be more than 1 StatisticsSearcher in the Searcher chain, each identified by a - * Searcher ID. The statistics accumulated by all StatisticsSearchers are stored - * in the singleton StatisticsManager object.

- *

- * TODO: Fix events to handle more than one of these searchers properly. + * Searcher ID. The statistics accumulated by all StatisticsSearchers are stored + * in the singleton StatisticsManager object. * * @author Gene Meyers * @author Steinar Knutsen * @author bergum */ -@Before(PhaseNames.RAW_QUERY) +@Before("*") public class StatisticsSearcher extends Searcher { private static final CompoundName IGNORE_QUERY = CompoundName.from("metrics.ignore"); @@ -89,39 +86,6 @@ private enum DegradedReason { match_phase, adaptive_timeout, timeout, non_ideal_ private final Map> relevanceContexts = new CopyOnWriteHashMap<>(); private final java.util.Timer scheduler = new java.util.Timer(true); - private class PeakQpsReporter extends java.util.TimerTask { - private long prevMaxQPSTime = System.currentTimeMillis(); - private long queriesForQPS = 0; - private Metric.Context metricContext = null; - public void setContext(Metric.Context metricContext) { - if (this.metricContext == null) { - synchronized(this) { - this.metricContext = metricContext; - } - } - } - @Override - public void run() { - long now = System.currentTimeMillis(); - synchronized (this) { - if (metricContext == null) return; - flushPeakQps(now); - } - } - private void flushPeakQps(long now) { - double ms = (double) (now - prevMaxQPSTime); - final double value = ((double)queriesForQPS) / (ms / 1000.0); - metric.set(PEAK_QPS_METRIC, value, metricContext); - prevMaxQPSTime = now; - queriesForQPS = 0; - } - void countQuery() { - synchronized (this) { - ++queriesForQPS; - } - } - } - public StatisticsSearcher(Metric metric, MetricReceiver metricReceiver) { this.peakQpsReporter = new PeakQpsReporter(); this.metric = metric; @@ -134,6 +98,72 @@ public StatisticsSearcher(Metric metric, MetricReceiver metricReceiver) { scheduler.schedule(peakQpsReporter, 1000, 1000); } + /** + * Generate statistics for the query passing through this Searcher + * 1) Add 1 to total query count + * 2) Add response time to total response time (time from entry to return) + * 3) ..... + */ + @Override + public Result search(Query query, Execution execution) { + if (query.properties().getBoolean(IGNORE_QUERY,false)) return execution.search(query); + + Metric.Context metricContext = getChainMetricContext(execution.chain().getId().stringValue()); + increaseQueryCount(metricContext); + logQuery(query); + long startMs = executionStartTime(query); + qps(metricContext); + metric.set(QUERY_TIMEOUT_METRIC, query.getTimeout(), metricContext); + + Result result; + try { + result = execution.search(query); + execution.fill(result); // Ensure the fill phase is included in latency measurements + } catch (Exception e) { + increaseErrorCount(null, metricContext); + throw e; + } + + if (startMs > 0) { + long endMs = System.currentTimeMillis(); + long latencyMs = endMs - startMs; + if (latencyMs >= 0) { + addLatency(latencyMs, metricContext); + } else { + getLogger().log(Level.WARNING, + "Apparently negative latency measure, start: " + startMs + + ", end: " + endMs + ", for query: " + query + ". Could be caused by NTP adjustments."); + } + } + if (result.hits().getError() != null) { + increaseErrorCount(result, metricContext); + incrementStatePageOnlyErrors(result); + } + Coverage queryCoverage = result.getCoverage(false); + if (queryCoverage != null) { + if (queryCoverage.isDegraded()) { + var degradedContext = getDegradedMetricContext(execution.chain().getId().stringValue(), queryCoverage); + metric.add(DEGRADED_QUERIES_METRIC, 1, degradedContext); + } + metric.add(DOCS_COVERED_METRIC, queryCoverage.getDocs(), metricContext); + metric.add(DOCS_TOTAL_METRIC, queryCoverage.getActive(), metricContext); + metric.add(DOCS_TARGET_TOTAL_METRIC, queryCoverage.getTargetActive(), metricContext); + } + int hitCount = result.getConcreteHitCount(); + metric.set(HITS_PER_QUERY_METRIC, (double) hitCount, metricContext); + + long totalHitCount = result.getTotalHitCount(); + metric.set(TOTALHITS_PER_QUERY_METRIC, (double) totalHitCount, metricContext); + metric.set(QUERY_HIT_OFFSET_METRIC, (double) (query.getHits() + query.getOffset()), metricContext); + if (hitCount == 0) { + metric.add(EMPTY_RESULTS_METRIC, 1, metricContext); + } + + addRelevanceMetrics(query, execution, result); + addItemCountMetric(query, metricContext); + return result; + } + @Override public void deconstruct() { scheduler.cancel(); @@ -207,77 +237,6 @@ private Metric.Context getRelevanceMetricContext(Execution execution, Query quer return metricContext; } - /** - * Generate statistics for the query passing through this Searcher - * 1) Add 1 to total query count - * 2) Add response time to total response time (time from entry to return) - * 3) ..... - */ - @Override - public Result search(Query query, Execution execution) { - if (query.properties().getBoolean(IGNORE_QUERY,false)) { - return execution.search(query); - } - - Metric.Context metricContext = getChainMetricContext(execution.chain().getId().stringValue()); - - incrQueryCount(metricContext); - logQuery(query); - long startMs = executionStartTime(query); - qps(metricContext); - metric.set(QUERY_TIMEOUT_METRIC, query.getTimeout(), metricContext); - Result result; - //handle exceptions thrown below in searchers - try { - result = execution.search(query); // Pass on down the chain - } catch (Exception e) { - incrErrorCount(null, metricContext); - throw e; - } - - if (startMs > 0) { - long endMs = System.currentTimeMillis(); - long latencyMs = endMs - startMs; - if (latencyMs >= 0) { - addLatency(latencyMs, metricContext); - } else { - getLogger().log(Level.WARNING, - "Apparently negative latency measure, start: " + startMs - + ", end: " + endMs + ", for query: " + query + ". Could be caused by NTP adjustments."); - } - } - if (result.hits().getError() != null) { - incrErrorCount(result, metricContext); - incrementStatePageOnlyErrors(result); - } - Coverage queryCoverage = result.getCoverage(false); - if (queryCoverage != null) { - if (queryCoverage.isDegraded()) { - Metric.Context degradedContext = getDegradedMetricContext(execution.chain().getId().stringValue(), queryCoverage); - metric.add(DEGRADED_QUERIES_METRIC, 1, degradedContext); - } - metric.add(DOCS_COVERED_METRIC, queryCoverage.getDocs(), metricContext); - metric.add(DOCS_TOTAL_METRIC, queryCoverage.getActive(), metricContext); - metric.add(DOCS_TARGET_TOTAL_METRIC, queryCoverage.getTargetActive(), metricContext); - } - int hitCount = result.getConcreteHitCount(); - metric.set(HITS_PER_QUERY_METRIC, (double) hitCount, metricContext); - - long totalHitCount = result.getTotalHitCount(); - metric.set(TOTALHITS_PER_QUERY_METRIC, (double) totalHitCount, metricContext); - metric.set(QUERY_HIT_OFFSET_METRIC, (double) (query.getHits() + query.getOffset()), metricContext); - if (hitCount == 0) { - metric.add(EMPTY_RESULTS_METRIC, 1, metricContext); - } - - addRelevanceMetrics(query, execution, result); - - addItemCountMetric(query, metricContext); - - return result; - } - - private void logQuery(com.yahoo.search.Query query) { // Don't parse the query if it's not necessary for the logging Query.toString triggers parsing if (getLogger().isLoggable(Level.FINER)) { @@ -291,11 +250,11 @@ private void addLatency(long latencyMs, Metric.Context metricContext) { metric.set(MAX_QUERY_LATENCY_METRIC, (double) latencyMs, metricContext); } - private void incrQueryCount(Metric.Context metricContext) { + private void increaseQueryCount(Metric.Context metricContext) { metric.add(QUERIES_METRIC, 1, metricContext); } - private void incrErrorCount(Result result, Metric.Context metricContext) { + private void increaseErrorCount(Result result, Metric.Context metricContext) { metric.add(FAILED_QUERIES_METRIC, 1, metricContext); if (result == null) // the chain threw an exception @@ -414,5 +373,44 @@ private static long executionStartTime(Query query) { return startTime != null ? (long) startTime : 0; } + private class PeakQpsReporter extends java.util.TimerTask { + + private long prevMaxQPSTime = System.currentTimeMillis(); + private long queriesForQPS = 0; + private Metric.Context metricContext = null; + + public void setContext(Metric.Context metricContext) { + if (this.metricContext == null) { + synchronized(this) { + this.metricContext = metricContext; + } + } + } + + @Override + public void run() { + long now = System.currentTimeMillis(); + synchronized (this) { + if (metricContext == null) return; + flushPeakQps(now); + } + } + + private void flushPeakQps(long now) { + double ms = (double) (now - prevMaxQPSTime); + final double value = ((double)queriesForQPS) / (ms / 1000.0); + metric.set(PEAK_QPS_METRIC, value, metricContext); + prevMaxQPSTime = now; + queriesForQPS = 0; + } + + void countQuery() { + synchronized (this) { + ++queriesForQPS; + } + } + + } + } diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/SearchChainRegistry.java b/container-search/src/main/java/com/yahoo/search/searchchain/SearchChainRegistry.java index 3fce9610eb39..9fd2efa7e46c 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/SearchChainRegistry.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/SearchChainRegistry.java @@ -61,7 +61,7 @@ private SearcherRegistry setupSearcherRegistry(ComponentRegistry searcherModel.bundleInstantiationSpec.id.getName().equals(requiredSearcher))); + } + +}