diff --git a/CHANGELOG.md b/CHANGELOG.md index 122caf2b49d7f..81d7dc20375e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added new Setting property UnmodifiableOnRestore to prevent updating settings on restore snapshot ([#16957](https://github.com/opensearch-project/OpenSearch/pull/16957)) - Introduce Template query ([#16818](https://github.com/opensearch-project/OpenSearch/pull/16818)) - Propagate the sourceIncludes and excludes fields from fetchSourceContext to FieldsVisitor. ([#17080](https://github.com/opensearch-project/OpenSearch/pull/17080)) +- Adds KNN related enums to support additional components for knn query during profiling ([#17146](https://github.com/opensearch-project/OpenSearch/pull/17146)) ### Dependencies - Bump `com.google.cloud:google-cloud-core-http` from 2.23.0 to 2.47.0 ([#16504](https://github.com/opensearch-project/OpenSearch/pull/16504)) diff --git a/server/src/main/java/org/opensearch/index/query/QueryBuilder.java b/server/src/main/java/org/opensearch/index/query/QueryBuilder.java index 0cdf7f31c2ebf..b79ef88e45098 100644 --- a/server/src/main/java/org/opensearch/index/query/QueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/QueryBuilder.java @@ -38,6 +38,8 @@ import org.opensearch.core.xcontent.ToXContentObject; import java.io.IOException; +import java.util.Collections; +import java.util.Set; /** * Foundation class for all OpenSearch query builders @@ -98,6 +100,10 @@ default QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOExc return this; } + default Set queryProfilerTimingTypes() { + return Collections.emptySet(); + } + /** * Recurse through the QueryBuilder tree, visiting any child QueryBuilder. * @param visitor a query builder visitor to be called by each query builder in the tree. diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index b84be7742394c..bf9f8d7d89087 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -34,6 +34,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.lucene.search.Query; import org.apache.lucene.util.Constants; import org.opensearch.Build; import org.opensearch.ExceptionsHelper; @@ -1386,7 +1387,8 @@ protected Node( circuitBreakerService, searchModule.getIndexSearcherExecutor(threadPool), taskResourceTrackingService, - searchModule.getConcurrentSearchRequestDeciderFactories() + searchModule.getConcurrentSearchRequestDeciderFactories(), + searchModule.getQueryProfilerTimingTypes() ); final List> tasksExecutors = pluginsService.filterPlugins(PersistentTaskPlugin.class) @@ -2050,7 +2052,8 @@ protected SearchService newSearchService( CircuitBreakerService circuitBreakerService, Executor indexSearcherExecutor, TaskResourceTrackingService taskResourceTrackingService, - Collection concurrentSearchDeciderFactories + Collection concurrentSearchDeciderFactories, + Map, Set> profilerTimingsPerQuery ) { return new SearchService( clusterService, @@ -2064,7 +2067,8 @@ protected SearchService newSearchService( circuitBreakerService, indexSearcherExecutor, taskResourceTrackingService, - concurrentSearchDeciderFactories + concurrentSearchDeciderFactories, + profilerTimingsPerQuery ); } diff --git a/server/src/main/java/org/opensearch/plugins/SearchPlugin.java b/server/src/main/java/org/opensearch/plugins/SearchPlugin.java index 60cb2184b5ab5..46fbdfcf5f331 100644 --- a/server/src/main/java/org/opensearch/plugins/SearchPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/SearchPlugin.java @@ -82,6 +82,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.TreeMap; import java.util.concurrent.ExecutorService; import java.util.function.BiFunction; @@ -227,6 +228,13 @@ default Optional getIndexSearcherExecutorProvider() { return Optional.empty(); } + /** + * Register any additional profiler timing types that is needed across all your queries + */ + default Map, Set> registerProfilerTimingTypes() { + return emptyMap(); + } + /** * Executor service provider */ diff --git a/server/src/main/java/org/opensearch/search/SearchModule.java b/server/src/main/java/org/opensearch/search/SearchModule.java index b8d3a13e0df20..596bff5fce5e3 100644 --- a/server/src/main/java/org/opensearch/search/SearchModule.java +++ b/server/src/main/java/org/opensearch/search/SearchModule.java @@ -33,6 +33,7 @@ package org.opensearch.search; import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.Query; import org.opensearch.common.NamedRegistry; import org.opensearch.common.Nullable; import org.opensearch.common.geo.GeoShapeType; @@ -300,9 +301,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.function.Consumer; import java.util.function.Function; @@ -335,6 +338,7 @@ public class SearchModule { private final SearchPlugin.ExecutorServiceProvider indexSearcherExecutorProvider; private final Collection concurrentSearchDeciderFactories; + private final Map, Set> profilerTimingsPerQuery = new HashMap<>(); /** * Constructs a new SearchModule object @@ -361,12 +365,19 @@ public SearchModule(Settings settings, List plugins) { registerSearchExts(plugins); registerShapes(); registerIntervalsSourceProviders(); + registerProfilerTimingTypes(plugins); queryPhaseSearcher = registerQueryPhaseSearcher(plugins); indexSearcherExecutorProvider = registerIndexSearcherExecutorProvider(plugins); namedWriteables.addAll(SortValue.namedWriteables()); concurrentSearchDeciderFactories = registerConcurrentSearchDeciderFactories(plugins); } + private void registerProfilerTimingTypes(List plugins) { + for (SearchPlugin plugin : plugins) { + profilerTimingsPerQuery.putAll(plugin.registerProfilerTimingTypes()); + } + } + private Collection registerConcurrentSearchDeciderFactories(List plugins) { List concurrentSearchDeciderFactories = new ArrayList<>(); for (SearchPlugin plugin : plugins) { @@ -383,6 +394,10 @@ public Collection getConcurrentSearchReq return concurrentSearchDeciderFactories; } + public Map, Set> getQueryProfilerTimingTypes() { + return profilerTimingsPerQuery; + } + public List getNamedWriteables() { return namedWriteables; } diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index b20f8222d6b7a..8e81e680dfd3f 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -36,6 +36,7 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Query; import org.apache.lucene.search.TopDocs; import org.opensearch.LegacyESVersion; import org.opensearch.OpenSearchException; @@ -401,6 +402,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final String sessionId = UUIDs.randomBase64UUID(); private final Executor indexSearcherExecutor; private final TaskResourceTrackingService taskResourceTrackingService; + private final Map, Set> profilerTimingsPerQuery; public SearchService( ClusterService clusterService, @@ -414,7 +416,8 @@ public SearchService( CircuitBreakerService circuitBreakerService, Executor indexSearcherExecutor, TaskResourceTrackingService taskResourceTrackingService, - Collection concurrentSearchDeciderFactories + Collection concurrentSearchDeciderFactories, + Map, Set> profilerTimingsPerQuery ) { Settings settings = clusterService.getSettings(); this.threadPool = threadPool; @@ -471,6 +474,7 @@ public SearchService( clusterService.getClusterSettings().addSettingsUpdateConsumer(CLUSTER_ALLOW_DERIVED_FIELD_SETTING, this::setAllowDerivedField); this.concurrentSearchDeciderFactories = concurrentSearchDeciderFactories; + this.profilerTimingsPerQuery = profilerTimingsPerQuery; } private void validateKeepAlives(TimeValue defaultKeepAlive, TimeValue maxKeepAlive) { @@ -1541,7 +1545,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc } context.evaluateRequestShouldUseConcurrentSearch(); if (source.profile()) { - context.setProfilers(new Profilers(context.searcher(), context.shouldUseConcurrentSearch())); + context.setProfilers(new Profilers(context.searcher(), context.shouldUseConcurrentSearch(), this.profilerTimingsPerQuery)); } if (this.indicesService.getCompositeIndexSettings() != null diff --git a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java index aa8212e8dad69..462b1ff83d05f 100644 --- a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java +++ b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java @@ -154,6 +154,10 @@ public void setProfiler(QueryProfiler profiler) { this.profiler = profiler; } + public QueryProfiler getProfiler() { + return profiler; + } + /** * Add a {@link Runnable} that will be run on a regular basis while accessing documents in the * DirectoryReader but also while collecting them and check for query cancellation or timeout. diff --git a/server/src/main/java/org/opensearch/search/profile/AbstractInternalProfileTree.java b/server/src/main/java/org/opensearch/search/profile/AbstractInternalProfileTree.java index 904b04b249b1b..3229948bf9880 100644 --- a/server/src/main/java/org/opensearch/search/profile/AbstractInternalProfileTree.java +++ b/server/src/main/java/org/opensearch/search/profile/AbstractInternalProfileTree.java @@ -39,6 +39,8 @@ import java.util.Collections; import java.util.Deque; import java.util.List; +import java.util.Map; +import java.util.Set; /** * Base class for a profiling tree. @@ -57,13 +59,16 @@ public abstract class AbstractInternalProfileTree stack; private int currentToken = 0; + /** Register of additional custom timings per element **/ + private Map, Set> profilerTimingsPerElement; - public AbstractInternalProfileTree() { + public AbstractInternalProfileTree(Map, Set> profilerTimingsPerElement) { breakdowns = new ArrayList<>(10); stack = new ArrayDeque<>(10); tree = new ArrayList<>(10); elements = new ArrayList<>(10); roots = new ArrayList<>(10); + this.profilerTimingsPerElement = profilerTimingsPerElement == null ? Collections.emptyMap() : profilerTimingsPerElement; } /** @@ -128,12 +133,17 @@ private PB addDependencyNode(E element, int token) { // Save our query for lookup later elements.add(element); - PB breakdown = createProfileBreakdown(); + final Set timings = profilerTimingsPerElement.get(element.getClass()); + PB breakdown = createProfileBreakdown(timings == null ? Collections.emptySet() : timings); breakdowns.add(token, breakdown); return breakdown; } - protected abstract PB createProfileBreakdown(); + public Map, Set> getProfilerTimingsPerElement() { + return profilerTimingsPerElement; + } + + protected abstract PB createProfileBreakdown(Set profilerTimingsPerElement); /** * Removes the last (e.g. most recent) value on the stack diff --git a/server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java b/server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java index 4a1563e7cdce9..732d98a58abbc 100644 --- a/server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java +++ b/server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java @@ -32,9 +32,16 @@ package org.opensearch.search.profile; +import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; +import java.util.Locale; import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeMap; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Collections.emptyMap; @@ -50,37 +57,41 @@ public abstract class AbstractProfileBreakdown> { /** * The accumulated timings for this query node */ - protected final Timer[] timings; - protected final T[] timingTypes; + protected final Map timings; public static final String TIMING_TYPE_COUNT_SUFFIX = "_count"; public static final String TIMING_TYPE_START_TIME_SUFFIX = "_start_time"; /** Sole constructor. */ - public AbstractProfileBreakdown(Class clazz) { - this.timingTypes = clazz.getEnumConstants(); - timings = new Timer[timingTypes.length]; - for (int i = 0; i < timings.length; ++i) { - timings[i] = new Timer(); - } + public AbstractProfileBreakdown(final Class timingType, final Set additionalProfilerTimings) { + Set additionalTimings = additionalProfilerTimings == null ? Collections.emptySet() : additionalProfilerTimings; + timings = Stream.of(Arrays.stream(timingType.getEnumConstants()).map(Enum::name), additionalTimings.stream()) + .flatMap(Function.identity()) + .filter(Objects::nonNull) + .map(val -> val.toLowerCase(Locale.ROOT)) + .collect(Collectors.toUnmodifiableMap(value -> value, value -> new Timer(), (a, b) -> a)); } public Timer getTimer(T timing) { - return timings[timing.ordinal()]; + return timings.get(timing.name().toLowerCase(Locale.ROOT)); + } + + public Timer getTimer(String timingName) { + return timings.get(timingName.toLowerCase(Locale.ROOT)); } public void setTimer(T timing, Timer timer) { - timings[timing.ordinal()] = timer; + timings.put(timing.name().toLowerCase(Locale.ROOT), timer); } /** * Build a timing count breakdown for current instance */ public Map toBreakdownMap() { - Map map = new HashMap<>(this.timings.length * 3); - for (T timingType : this.timingTypes) { - map.put(timingType.toString(), this.timings[timingType.ordinal()].getApproximateTiming()); - map.put(timingType + TIMING_TYPE_COUNT_SUFFIX, this.timings[timingType.ordinal()].getCount()); - map.put(timingType + TIMING_TYPE_START_TIME_SUFFIX, this.timings[timingType.ordinal()].getEarliestTimerStartTime()); + Map map = new TreeMap<>(); + for (String timingType : this.timings.keySet()) { + map.put(timingType, this.timings.get(timingType).getApproximateTiming()); + map.put(timingType + TIMING_TYPE_COUNT_SUFFIX, this.timings.get(timingType).getCount()); + map.put(timingType + TIMING_TYPE_START_TIME_SUFFIX, this.timings.get(timingType).getEarliestTimerStartTime()); } return Collections.unmodifiableMap(map); } @@ -94,8 +105,8 @@ public Map toDebugMap() { public long toNodeTime() { long total = 0; - for (T timingType : timingTypes) { - total += timings[timingType.ordinal()].getApproximateTiming(); + for (String timingType : timings.keySet()) { + total += timings.get(timingType).getApproximateTiming(); } return total; } diff --git a/server/src/main/java/org/opensearch/search/profile/ContextualProfileBreakdown.java b/server/src/main/java/org/opensearch/search/profile/ContextualProfileBreakdown.java index 3fe621321c8ad..1ff969ed606bb 100644 --- a/server/src/main/java/org/opensearch/search/profile/ContextualProfileBreakdown.java +++ b/server/src/main/java/org/opensearch/search/profile/ContextualProfileBreakdown.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; +import java.util.Set; /** * Provide contextual profile breakdowns which are associated with freestyle context. Used when concurrent @@ -21,8 +22,8 @@ * @opensearch.internal */ public abstract class ContextualProfileBreakdown> extends AbstractProfileBreakdown { - public ContextualProfileBreakdown(Class clazz) { - super(clazz); + public ContextualProfileBreakdown(Class clazz, Set additionalProfilerTimings) { + super(clazz, additionalProfilerTimings); } /** diff --git a/server/src/main/java/org/opensearch/search/profile/Profilers.java b/server/src/main/java/org/opensearch/search/profile/Profilers.java index 75337f89e67ca..46b5a38921ac5 100644 --- a/server/src/main/java/org/opensearch/search/profile/Profilers.java +++ b/server/src/main/java/org/opensearch/search/profile/Profilers.java @@ -32,6 +32,7 @@ package org.opensearch.search.profile; +import org.apache.lucene.search.Query; import org.opensearch.common.annotation.PublicApi; import org.opensearch.search.internal.ContextIndexSearcher; import org.opensearch.search.profile.aggregation.AggregationProfiler; @@ -44,6 +45,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.Set; /** * Wrapper around all the profilers that makes management easier. @@ -59,19 +62,28 @@ public final class Profilers { private final boolean isConcurrentSegmentSearchEnabled; /** Sole constructor. This {@link Profilers} instance will initially wrap one {@link QueryProfiler}. */ - public Profilers(ContextIndexSearcher searcher, boolean isConcurrentSegmentSearchEnabled) { + public Profilers( + ContextIndexSearcher searcher, + boolean isConcurrentSegmentSearchEnabled, + Map, Set> profileTimingsPerQuery + ) { this.searcher = searcher; this.isConcurrentSegmentSearchEnabled = isConcurrentSegmentSearchEnabled; this.queryProfilers = new ArrayList<>(); this.aggProfiler = isConcurrentSegmentSearchEnabled ? new ConcurrentAggregationProfiler() : new AggregationProfiler(); - addQueryProfiler(); + addQueryProfiler(profileTimingsPerQuery); } /** Switch to a new profile. */ public QueryProfiler addQueryProfiler() { + return addQueryProfiler(Collections.emptyMap()); + } + + /** Switch to a new profile. */ + public QueryProfiler addQueryProfiler(final Map, Set> profileTimingsPerQuery) { QueryProfiler profiler = isConcurrentSegmentSearchEnabled - ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree()) - : new QueryProfiler(new InternalQueryProfileTree()); + ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree(profileTimingsPerQuery)) + : new QueryProfiler(new InternalQueryProfileTree(profileTimingsPerQuery)); searcher.setProfiler(profiler); queryProfilers.add(profiler); return profiler; diff --git a/server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java b/server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java index 8642f0da4a90b..01f063ced7f23 100644 --- a/server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java +++ b/server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java @@ -38,6 +38,7 @@ import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableMap; /** @@ -50,7 +51,7 @@ public class AggregationProfileBreakdown extends AbstractProfileBreakdown extra = new HashMap<>(); public AggregationProfileBreakdown() { - super(AggregationTimingType.class); + super(AggregationTimingType.class, emptySet()); } /** diff --git a/server/src/main/java/org/opensearch/search/profile/aggregation/InternalAggregationProfileTree.java b/server/src/main/java/org/opensearch/search/profile/aggregation/InternalAggregationProfileTree.java index 34716b87c7c9c..264ca0eb87eb7 100644 --- a/server/src/main/java/org/opensearch/search/profile/aggregation/InternalAggregationProfileTree.java +++ b/server/src/main/java/org/opensearch/search/profile/aggregation/InternalAggregationProfileTree.java @@ -35,6 +35,9 @@ import org.opensearch.search.aggregations.Aggregator; import org.opensearch.search.profile.AbstractInternalProfileTree; +import java.util.Collections; +import java.util.Set; + /** * The profiling tree for different levels of agg profiling * @@ -42,8 +45,12 @@ */ public class InternalAggregationProfileTree extends AbstractInternalProfileTree { + public InternalAggregationProfileTree() { + super(Collections.emptyMap()); + } + @Override - protected AggregationProfileBreakdown createProfileBreakdown() { + protected AggregationProfileBreakdown createProfileBreakdown(Set additionalProfilerTimings) { return new AggregationProfileBreakdown(); } diff --git a/server/src/main/java/org/opensearch/search/profile/query/AbstractQueryProfileTree.java b/server/src/main/java/org/opensearch/search/profile/query/AbstractQueryProfileTree.java index 2f5d632ee2d87..cb28561ecc655 100644 --- a/server/src/main/java/org/opensearch/search/profile/query/AbstractQueryProfileTree.java +++ b/server/src/main/java/org/opensearch/search/profile/query/AbstractQueryProfileTree.java @@ -13,6 +13,9 @@ import org.opensearch.search.profile.ContextualProfileBreakdown; import org.opensearch.search.profile.ProfileResult; +import java.util.Map; +import java.util.Set; + /** * This class tracks the dependency tree for queries (scoring and rewriting) and * generates {@link QueryProfileBreakdown} for each node in the tree. It also finalizes the tree @@ -26,6 +29,10 @@ public abstract class AbstractQueryProfileTree extends AbstractInternalProfileTr private long rewriteTime; private long rewriteScratch; + protected AbstractQueryProfileTree(Map, Set> profilerTimingsPerQuery) { + super(profilerTimingsPerQuery); + } + @Override protected String getTypeFromElement(Query query) { // Anonymous classes won't have a name, diff --git a/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java b/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java index 99169b42c05f0..e966432a433d3 100644 --- a/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java +++ b/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java @@ -18,7 +18,10 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; /** @@ -44,10 +47,12 @@ public final class ConcurrentQueryProfileBreakdown extends ContextualProfileBrea // represents slice to leaves mapping as for each slice a unique collector instance is created private final Map> sliceCollectorsToLeaves = new ConcurrentHashMap<>(); + private final Set additionalProfilerTimings; /** Sole constructor. */ - public ConcurrentQueryProfileBreakdown() { - super(QueryTimingType.class); + public ConcurrentQueryProfileBreakdown(Set additionalProfilerTimings) { + super(QueryTimingType.class, additionalProfilerTimings); + this.additionalProfilerTimings = additionalProfilerTimings; } @Override @@ -59,16 +64,16 @@ public AbstractProfileBreakdown context(Object context) { return profile; } - return contexts.computeIfAbsent(context, ctx -> new QueryProfileBreakdown()); + return contexts.computeIfAbsent(context, ctx -> new QueryProfileBreakdown(additionalProfilerTimings)); } @Override public Map toBreakdownMap() { final Map topLevelBreakdownMapWithWeightTime = super.toBreakdownMap(); final long createWeightStartTime = topLevelBreakdownMapWithWeightTime.get( - QueryTimingType.CREATE_WEIGHT + TIMING_TYPE_START_TIME_SUFFIX + QueryTimingType.CREATE_WEIGHT.name().toLowerCase(Locale.ROOT) + TIMING_TYPE_START_TIME_SUFFIX ); - final long createWeightTime = topLevelBreakdownMapWithWeightTime.get(QueryTimingType.CREATE_WEIGHT.toString()); + final long createWeightTime = topLevelBreakdownMapWithWeightTime.get(QueryTimingType.CREATE_WEIGHT.name().toLowerCase(Locale.ROOT)); if (contexts.isEmpty()) { // If there are no leaf contexts, then return the default concurrent query level breakdown, which will include the @@ -92,9 +97,9 @@ public Map toBreakdownMap() { maxSliceNodeTime = 0L; minSliceNodeTime = 0L; avgSliceNodeTime = 0L; - Map queryBreakdownMap = new HashMap<>(breakdown.toBreakdownMap()); - queryBreakdownMap.put(QueryTimingType.CREATE_WEIGHT.toString(), createWeightTime); - queryBreakdownMap.put(QueryTimingType.CREATE_WEIGHT + TIMING_TYPE_COUNT_SUFFIX, 1L); + Map queryBreakdownMap = new TreeMap<>(breakdown.toBreakdownMap()); + queryBreakdownMap.put(QueryTimingType.CREATE_WEIGHT.name().toLowerCase(Locale.ROOT), createWeightTime); + queryBreakdownMap.put(QueryTimingType.CREATE_WEIGHT.name().toLowerCase(Locale.ROOT) + TIMING_TYPE_COUNT_SUFFIX, 1L); return queryBreakdownMap; } @@ -110,12 +115,11 @@ public Map toBreakdownMap() { * default breakdown map. */ private Map buildDefaultQueryBreakdownMap(long createWeightTime) { - final Map concurrentQueryBreakdownMap = new HashMap<>(); - for (QueryTimingType timingType : QueryTimingType.values()) { - final String timingTypeKey = timingType.toString(); + final Map concurrentQueryBreakdownMap = new TreeMap<>(); + for (String timingTypeKey : timings.keySet()) { final String timingTypeCountKey = timingTypeKey + TIMING_TYPE_COUNT_SUFFIX; - if (timingType.equals(QueryTimingType.CREATE_WEIGHT)) { + if (timingTypeKey.equalsIgnoreCase(QueryTimingType.CREATE_WEIGHT.name())) { concurrentQueryBreakdownMap.put(timingTypeKey, createWeightTime); concurrentQueryBreakdownMap.put(timingTypeCountKey, 1L); continue; @@ -156,8 +160,8 @@ Map> buildSliceLevelBreakdown() { // max slice end time across all timing types long sliceMaxEndTime = Long.MIN_VALUE; long sliceMinStartTime = Long.MAX_VALUE; - for (QueryTimingType timingType : QueryTimingType.values()) { - if (timingType.equals(QueryTimingType.CREATE_WEIGHT)) { + for (String timingType : timings.keySet()) { + if (timingType.equalsIgnoreCase(QueryTimingType.CREATE_WEIGHT.name())) { // do nothing for create weight as that is query level time and not slice level continue; } @@ -210,9 +214,7 @@ Map> buildSliceLevelBreakdown() { ); // compute the sliceEndTime for timingType using max of endTime across slice leaves - final long sliceLeafTimingTypeEndTime = sliceLeafTimingTypeStartTime + currentSliceLeafBreakdownMap.get( - timingType.toString() - ); + final long sliceLeafTimingTypeEndTime = sliceLeafTimingTypeStartTime + currentSliceLeafBreakdownMap.get(timingType); currentSliceBreakdown.compute( timingTypeSliceEndTimeKey, (key, value) -> (value == null) ? sliceLeafTimingTypeEndTime : Math.max(value, sliceLeafTimingTypeEndTime) @@ -235,7 +237,7 @@ Map> buildSliceLevelBreakdown() { sliceMinStartTime = Math.min(sliceMinStartTime, currentSliceStartTime); // compute total time for each timing type at slice level using sliceEndTime and sliceStartTime currentSliceBreakdown.put( - timingType.toString(), + timingType, currentSliceBreakdown.getOrDefault(timingTypeSliceEndTimeKey, 0L) - currentSliceBreakdown.getOrDefault( timingTypeSliceStartTimeKey, 0L @@ -284,10 +286,9 @@ public Map buildQueryBreakdownMap( long createWeightTime, long createWeightStartTime ) { - final Map queryBreakdownMap = new HashMap<>(); + final Map queryBreakdownMap = new TreeMap<>(); long queryEndTime = Long.MIN_VALUE; - for (QueryTimingType queryTimingType : QueryTimingType.values()) { - final String timingTypeKey = queryTimingType.toString(); + for (String timingTypeKey : timings.keySet()) { final String timingTypeCountKey = timingTypeKey + TIMING_TYPE_COUNT_SUFFIX; final String sliceEndTimeForTimingType = timingTypeKey + SLICE_END_TIME_SUFFIX; final String sliceStartTimeForTimingType = timingTypeKey + SLICE_START_TIME_SUFFIX; @@ -304,7 +305,7 @@ public Map buildQueryBreakdownMap( long queryTimingTypeCount = 0L; // the create weight time is computed at the query level and is called only once per query - if (queryTimingType == QueryTimingType.CREATE_WEIGHT) { + if (timingTypeKey.equalsIgnoreCase(QueryTimingType.CREATE_WEIGHT.name())) { queryBreakdownMap.put(timingTypeCountKey, 1L); queryBreakdownMap.put(timingTypeKey, createWeightTime); continue; diff --git a/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileTree.java b/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileTree.java index 4e54178c3b4fb..f12cef5d4de92 100644 --- a/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileTree.java +++ b/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileTree.java @@ -10,11 +10,13 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.Collector; +import org.apache.lucene.search.Query; import org.opensearch.search.profile.ContextualProfileBreakdown; import org.opensearch.search.profile.ProfileResult; import java.util.List; import java.util.Map; +import java.util.Set; /** * This class returns a list of {@link ProfileResult} that can be serialized back to the client in the concurrent execution. @@ -23,9 +25,13 @@ */ public class ConcurrentQueryProfileTree extends AbstractQueryProfileTree { + public ConcurrentQueryProfileTree(Map, Set> profilerTimingsPerQuery) { + super(profilerTimingsPerQuery); + } + @Override - protected ContextualProfileBreakdown createProfileBreakdown() { - return new ConcurrentQueryProfileBreakdown(); + protected ContextualProfileBreakdown createProfileBreakdown(Set additionalProfilerTimings) { + return new ConcurrentQueryProfileBreakdown(additionalProfilerTimings); } @Override diff --git a/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfiler.java b/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfiler.java index 42bf23bb13fbe..8359b02bc36af 100644 --- a/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfiler.java +++ b/server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfiler.java @@ -48,7 +48,7 @@ public ConcurrentQueryProfiler(AbstractQueryProfileTree profileTree) { public ContextualProfileBreakdown getQueryBreakdown(Query query) { ConcurrentQueryProfileTree profileTree = threadToProfileTree.computeIfAbsent( getCurrentThreadId(), - k -> new ConcurrentQueryProfileTree() + k -> new ConcurrentQueryProfileTree(super.profileTree.getProfilerTimingsPerElement()) ); return profileTree.getProfileBreakdown(query); } diff --git a/server/src/main/java/org/opensearch/search/profile/query/InternalQueryProfileTree.java b/server/src/main/java/org/opensearch/search/profile/query/InternalQueryProfileTree.java index 1ed367f094fb7..493eb9999832a 100644 --- a/server/src/main/java/org/opensearch/search/profile/query/InternalQueryProfileTree.java +++ b/server/src/main/java/org/opensearch/search/profile/query/InternalQueryProfileTree.java @@ -32,9 +32,13 @@ package org.opensearch.search.profile.query; +import org.apache.lucene.search.Query; import org.opensearch.search.profile.ContextualProfileBreakdown; import org.opensearch.search.profile.ProfileResult; +import java.util.Map; +import java.util.Set; + /** * This class returns a list of {@link ProfileResult} that can be serialized back to the client in the non-concurrent execution. * @@ -42,8 +46,12 @@ */ public class InternalQueryProfileTree extends AbstractQueryProfileTree { + public InternalQueryProfileTree(final Map, Set> profilerTimingsPerQuery) { + super(profilerTimingsPerQuery); + } + @Override - protected ContextualProfileBreakdown createProfileBreakdown() { - return new QueryProfileBreakdown(); + protected ContextualProfileBreakdown createProfileBreakdown(Set additionalProfilerTimings) { + return new QueryProfileBreakdown(additionalProfilerTimings); } } diff --git a/server/src/main/java/org/opensearch/search/profile/query/QueryProfileBreakdown.java b/server/src/main/java/org/opensearch/search/profile/query/QueryProfileBreakdown.java index 3514a80e39d85..1b786ed79f838 100644 --- a/server/src/main/java/org/opensearch/search/profile/query/QueryProfileBreakdown.java +++ b/server/src/main/java/org/opensearch/search/profile/query/QueryProfileBreakdown.java @@ -35,6 +35,8 @@ import org.opensearch.search.profile.AbstractProfileBreakdown; import org.opensearch.search.profile.ContextualProfileBreakdown; +import java.util.Set; + /** * A record of timings for the various operations that may happen during query execution. * A node's time may be composed of several internal attributes (rewriting, weighting, @@ -45,8 +47,8 @@ public final class QueryProfileBreakdown extends ContextualProfileBreakdown { /** Sole constructor. */ - public QueryProfileBreakdown() { - super(QueryTimingType.class); + public QueryProfileBreakdown(Set additionalPluginTimings) { + super(QueryTimingType.class, additionalPluginTimings); } @Override diff --git a/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdownTests.java b/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdownTests.java index db14eb90ef839..e475ffc135f81 100644 --- a/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdownTests.java +++ b/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdownTests.java @@ -30,6 +30,7 @@ import org.opensearch.test.OpenSearchTestCase; import org.junit.Before; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -46,7 +47,7 @@ public class ConcurrentQueryProfileBreakdownTests extends OpenSearchTestCase { @Before public void setup() { - testQueryProfileBreakdown = new ConcurrentQueryProfileBreakdown(); + testQueryProfileBreakdown = new ConcurrentQueryProfileBreakdown(Collections.emptySet()); createWeightTimer = testQueryProfileBreakdown.getTimer(QueryTimingType.CREATE_WEIGHT); try { createWeightTimer.start(); @@ -420,7 +421,7 @@ private static class TestQueryProfileBreakdown extends AbstractProfileBreakdown< private Map breakdownMap; public TestQueryProfileBreakdown(Class clazz, Map breakdownMap) { - super(clazz); + super(clazz, Collections.emptySet()); this.breakdownMap = breakdownMap; } diff --git a/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfilerTests.java b/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfilerTests.java index 736bbcdd9e8dd..4e9f8b2f6f559 100644 --- a/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfilerTests.java +++ b/server/src/test/java/org/opensearch/search/profile/query/ConcurrentQueryProfilerTests.java @@ -11,6 +11,7 @@ import org.opensearch.search.profile.Timer; import org.opensearch.test.OpenSearchTestCase; +import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -19,7 +20,7 @@ public class ConcurrentQueryProfilerTests extends OpenSearchTestCase { public void testMergeRewriteTimeIntervals() { - ConcurrentQueryProfiler profiler = new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree()); + ConcurrentQueryProfiler profiler = new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree(Collections.emptySet())); List timers = new LinkedList<>(); timers.add(new Timer(217134L, 1L, 1L, 0L, 553074511206907L)); timers.add(new Timer(228954L, 1L, 1L, 0L, 553074509287335L)); diff --git a/server/src/test/java/org/opensearch/search/profile/query/ProfileScorerTests.java b/server/src/test/java/org/opensearch/search/profile/query/ProfileScorerTests.java index 5b2cb3e17fb38..ff826f6f5d1c2 100644 --- a/server/src/test/java/org/opensearch/search/profile/query/ProfileScorerTests.java +++ b/server/src/test/java/org/opensearch/search/profile/query/ProfileScorerTests.java @@ -43,6 +43,7 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.util.Collections; public class ProfileScorerTests extends OpenSearchTestCase { @@ -84,7 +85,7 @@ public void testPropagateMinCompetitiveScore() throws IOException { Query query = new MatchAllDocsQuery(); Weight weight = query.createWeight(new IndexSearcher(new MultiReader()), ScoreMode.TOP_SCORES, 1f); FakeScorer fakeScorer = new FakeScorer(weight); - QueryProfileBreakdown profile = new QueryProfileBreakdown(); + QueryProfileBreakdown profile = new QueryProfileBreakdown(Collections.emptySet()); ProfileWeight profileWeight = new ProfileWeight(query, weight, profile); ProfileScorer profileScorer = new ProfileScorer(profileWeight, fakeScorer, profile); profileScorer.setMinCompetitiveScore(0.42f); @@ -95,7 +96,7 @@ public void testPropagateMaxScore() throws IOException { Query query = new MatchAllDocsQuery(); Weight weight = query.createWeight(new IndexSearcher(new MultiReader()), ScoreMode.TOP_SCORES, 1f); FakeScorer fakeScorer = new FakeScorer(weight); - QueryProfileBreakdown profile = new QueryProfileBreakdown(); + QueryProfileBreakdown profile = new QueryProfileBreakdown(Collections.emptySet()); ProfileWeight profileWeight = new ProfileWeight(query, weight, profile); ProfileScorer profileScorer = new ProfileScorer(profileWeight, fakeScorer, profile); profileScorer.setMinCompetitiveScore(0.42f); diff --git a/server/src/test/java/org/opensearch/search/profile/query/QueryProfilerTests.java b/server/src/test/java/org/opensearch/search/profile/query/QueryProfilerTests.java index 3a7c711d324c4..24a4be5d3e296 100644 --- a/server/src/test/java/org/opensearch/search/profile/query/QueryProfilerTests.java +++ b/server/src/test/java/org/opensearch/search/profile/query/QueryProfilerTests.java @@ -74,6 +74,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; @@ -166,8 +167,8 @@ public void tearDown() throws Exception { public void testBasic() throws IOException { QueryProfiler profiler = executor != null - ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree()) - : new QueryProfiler(new InternalQueryProfileTree()); + ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree(Collections.emptySet())) + : new QueryProfiler(new InternalQueryProfileTree(Collections.emptySet())); searcher.setProfiler(profiler); Query query = new TermQuery(new Term("foo", "bar")); searcher.search(query, 1); @@ -235,8 +236,8 @@ public void testBasic() throws IOException { public void testNoScoring() throws IOException { QueryProfiler profiler = executor != null - ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree()) - : new QueryProfiler(new InternalQueryProfileTree()); + ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree(Collections.emptySet())) + : new QueryProfiler(new InternalQueryProfileTree(Collections.emptySet())); searcher.setProfiler(profiler); Query query = new TermQuery(new Term("foo", "bar")); searcher.search(query, 1, Sort.INDEXORDER); // scores are not needed @@ -304,8 +305,8 @@ public void testNoScoring() throws IOException { public void testUseIndexStats() throws IOException { QueryProfiler profiler = executor != null - ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree()) - : new QueryProfiler(new InternalQueryProfileTree()); + ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree(Collections.emptySet())) + : new QueryProfiler(new InternalQueryProfileTree(Collections.emptySet())); searcher.setProfiler(profiler); Query query = new TermQuery(new Term("foo", "bar")); searcher.count(query); // will use index stats @@ -320,8 +321,8 @@ public void testUseIndexStats() throws IOException { public void testApproximations() throws IOException { QueryProfiler profiler = executor != null - ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree()) - : new QueryProfiler(new InternalQueryProfileTree()); + ? new ConcurrentQueryProfiler(new ConcurrentQueryProfileTree(Collections.emptySet())) + : new QueryProfiler(new InternalQueryProfileTree(Collections.emptySet())); searcher.setProfiler(profiler); Query query = new RandomApproximationQuery(new TermQuery(new Term("foo", "bar")), random()); searcher.count(query); diff --git a/test/framework/src/main/java/org/opensearch/node/MockNode.java b/test/framework/src/main/java/org/opensearch/node/MockNode.java index 97c06962ca2e7..5b5d41f84d157 100644 --- a/test/framework/src/main/java/org/opensearch/node/MockNode.java +++ b/test/framework/src/main/java/org/opensearch/node/MockNode.java @@ -32,6 +32,7 @@ package org.opensearch.node; +import org.apache.lucene.search.Query; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterInfoService; import org.opensearch.cluster.MockInternalClusterInfoService; @@ -158,7 +159,8 @@ protected SearchService newSearchService( CircuitBreakerService circuitBreakerService, Executor indexSearcherExecutor, TaskResourceTrackingService taskResourceTrackingService, - Collection concurrentSearchDeciderFactories + Collection concurrentSearchDeciderFactories, + Map, Set> profilerTimingsPerQuery ) { if (getPluginsService().filterPlugins(MockSearchService.TestPlugin.class).isEmpty()) { return super.newSearchService( @@ -173,7 +175,8 @@ protected SearchService newSearchService( circuitBreakerService, indexSearcherExecutor, taskResourceTrackingService, - concurrentSearchDeciderFactories + concurrentSearchDeciderFactories, + Collections.emptyMap() ); } return new MockSearchService( diff --git a/test/framework/src/main/java/org/opensearch/search/MockSearchService.java b/test/framework/src/main/java/org/opensearch/search/MockSearchService.java index 28e202e783c4e..a1f2925c0e31a 100644 --- a/test/framework/src/main/java/org/opensearch/search/MockSearchService.java +++ b/test/framework/src/main/java/org/opensearch/search/MockSearchService.java @@ -113,7 +113,8 @@ public MockSearchService( circuitBreakerService, indexSearcherExecutor, taskResourceTrackingService, - Collections.emptyList() + Collections.emptyList(), + Collections.emptyMap() ); } diff --git a/test/framework/src/main/java/org/opensearch/test/TestSearchContext.java b/test/framework/src/main/java/org/opensearch/test/TestSearchContext.java index cbfeea4633ad6..a47873aa4b9fc 100644 --- a/test/framework/src/main/java/org/opensearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/opensearch/test/TestSearchContext.java @@ -732,7 +732,7 @@ public TestSearchContext withCleanQueryResult() { * Add profilers to the query */ public TestSearchContext withProfilers() { - this.profilers = new Profilers(searcher, concurrentSegmentSearchEnabled); + this.profilers = new Profilers(searcher, concurrentSegmentSearchEnabled, Collections.emptyMap()); return this; } }