diff --git a/CHANGELOG.md b/CHANGELOG.md index 9aada4ab95379..80e944260ca41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add task cancellation timestamp in task API ([#7455](https://github.com/opensearch-project/OpenSearch/pull/7455)) - Adds ExtensionsManager.lookupExtensionSettingsById ([#7466](https://github.com/opensearch-project/OpenSearch/pull/7466)) - SegRep with Remote: Add hook for publishing checkpoint notifications after segment upload to remote store ([#7394](https://github.com/opensearch-project/OpenSearch/pull/7394)) +- Add search_after query optimizations with shard/segment short cutting ([#7453](https://github.com/opensearch-project/OpenSearch/pull/7453)) - Provide mechanism to configure XContent parsing constraints (after update to Jackson 2.15.0 and above) ([#7550](https://github.com/opensearch-project/OpenSearch/pull/7550)) - Support to clear filecache using clear indices cache API ([#7498](https://github.com/opensearch-project/OpenSearch/pull/7498)) - Create NamedRoute to map extension routes to a shortened name ([#6870](https://github.com/opensearch-project/OpenSearch/pull/6870)) diff --git a/server/src/main/java/org/opensearch/search/DefaultSearchContext.java b/server/src/main/java/org/opensearch/search/DefaultSearchContext.java index 1bde741973983..40081c087f09a 100644 --- a/server/src/main/java/org/opensearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/opensearch/search/DefaultSearchContext.java @@ -89,7 +89,6 @@ import org.opensearch.search.rescore.RescoreContext; import org.opensearch.search.slice.SliceBuilder; import org.opensearch.search.sort.SortAndFormats; -import org.opensearch.search.sort.SortOrder; import org.opensearch.search.suggest.SuggestionSearchContext; import java.io.IOException; @@ -212,7 +211,7 @@ final class DefaultSearchContext extends SearchContext { engineSearcher.getQueryCachingPolicy(), lowLevelCancellation, executor, - shouldReverseLeafReaderContexts() + this ); this.relativeTimeSupplier = relativeTimeSupplier; this.timeout = timeout; @@ -887,22 +886,4 @@ public boolean isCancelled() { public ReaderContext readerContext() { return readerContext; } - - private boolean shouldReverseLeafReaderContexts() { - // Time series based workload by default traverses segments in desc order i.e. latest to the oldest order. - // This is actually beneficial for search queries to start search on latest segments first for time series workload. - // That can slow down ASC order queries on timestamp workload. So to avoid that slowdown, we will reverse leaf - // reader order here. - if (this.indexShard.isTimeSeriesIndex()) { - // Only reverse order for asc order sort queries - if (request != null - && request.source() != null - && request.source().sorts() != null - && request.source().sorts().size() > 0 - && request.source().sorts().get(0).order() == SortOrder.ASC) { - return true; - } - } - return false; - } } diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index cdd30a2a8847d..efb5800879495 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -133,6 +133,7 @@ import org.opensearch.search.sort.MinAndMax; import org.opensearch.search.sort.SortAndFormats; import org.opensearch.search.sort.SortBuilder; +import org.opensearch.search.sort.SortOrder; import org.opensearch.search.suggest.Suggest; import org.opensearch.search.suggest.completion.CompletionSuggestion; import org.opensearch.threadpool.Scheduler.Cancellable; @@ -1525,7 +1526,7 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre final boolean aliasFilterCanMatch = request.getAliasFilter().getQueryBuilder() instanceof MatchNoneQueryBuilder == false; FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source()); MinAndMax minMax = sortBuilder != null ? FieldSortBuilder.getMinMaxOrNull(context, sortBuilder) : null; - final boolean canMatch; + boolean canMatch; if (canRewriteToMatchNone(request.source())) { QueryBuilder queryBuilder = request.source().query(); canMatch = aliasFilterCanMatch && queryBuilder instanceof MatchNoneQueryBuilder == false; @@ -1533,11 +1534,44 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre // null query means match_all canMatch = aliasFilterCanMatch; } + final FieldDoc searchAfterFieldDoc = getSearchAfterFieldDoc(request, context); + canMatch = canMatch && canMatchSearchAfter(searchAfterFieldDoc, minMax, sortBuilder); + return new CanMatchResponse(canMatch || hasRefreshPending, minMax); } } } + public static boolean canMatchSearchAfter(FieldDoc searchAfter, MinAndMax minMax, FieldSortBuilder primarySortField) { + if (searchAfter != null && minMax != null && primarySortField != null) { + final Object searchAfterPrimary = searchAfter.fields[0]; + if (primarySortField.order() == SortOrder.DESC) { + if (minMax.compareMin(searchAfterPrimary) > 0) { + // In Desc order, if segment/shard minimum is gt search_after, the segment/shard won't be competitive + return false; + } + } else { + if (minMax.compareMax(searchAfterPrimary) < 0) { + // In ASC order, if segment/shard maximum is lt search_after, the segment/shard won't be competitive + return false; + } + } + } + return true; + } + + private static FieldDoc getSearchAfterFieldDoc(ShardSearchRequest request, QueryShardContext context) throws IOException { + if (context != null && request != null && request.source() != null && request.source().sorts() != null) { + final List> sorts = request.source().sorts(); + final Object[] searchAfter = request.source().searchAfter(); + final Optional sortOpt = SortBuilder.buildSort(sorts, context); + if (sortOpt.isPresent() && !CollectionUtils.isEmpty(searchAfter)) { + return SearchAfterBuilder.buildFieldDoc(sortOpt.get(), searchAfter); + } + } + return null; + } + /** * Returns true iff the given search source builder can be early terminated by rewriting to a match none query. Or in other words * if the execution of the search request can be early terminated without executing it. This is for instance not possible if 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 818da075fd7ec..79734b1e25005 100644 --- a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java +++ b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java @@ -65,6 +65,7 @@ import org.opensearch.common.lease.Releasable; import org.opensearch.common.lucene.search.TopDocsAndMaxScore; import org.opensearch.search.DocValueFormat; +import org.opensearch.search.SearchService; import org.opensearch.search.dfs.AggregatedDfs; import org.opensearch.search.profile.ContextualProfileBreakdown; import org.opensearch.search.profile.Timer; @@ -72,6 +73,9 @@ import org.opensearch.search.profile.query.QueryProfiler; import org.opensearch.search.profile.query.QueryTimingType; import org.opensearch.search.query.QuerySearchResult; +import org.opensearch.search.sort.FieldSortBuilder; +import org.opensearch.search.sort.MinAndMax; +import org.opensearch.search.sort.SortOrder; import java.io.IOException; import java.util.ArrayList; @@ -97,12 +101,7 @@ public class ContextIndexSearcher extends IndexSearcher implements Releasable { private AggregatedDfs aggregatedDfs; private QueryProfiler profiler; private MutableQueryTimeout cancellable; - - /** - * Certain queries can benefit if we reverse the segment read order, - * for example time series based queries if searched for desc sort order - */ - private final boolean reverseLeafReaderContexts; + private SearchContext searchContext; public ContextIndexSearcher( IndexReader reader, @@ -120,7 +119,7 @@ public ContextIndexSearcher( new MutableQueryTimeout(), wrapWithExitableDirectoryReader, executor, - false + null ); } @@ -131,7 +130,7 @@ public ContextIndexSearcher( QueryCachingPolicy queryCachingPolicy, boolean wrapWithExitableDirectoryReader, Executor executor, - boolean reverseLeafReaderContexts + SearchContext searchContext ) throws IOException { this( reader, @@ -141,7 +140,7 @@ public ContextIndexSearcher( new MutableQueryTimeout(), wrapWithExitableDirectoryReader, executor, - reverseLeafReaderContexts + searchContext ); } @@ -153,14 +152,14 @@ private ContextIndexSearcher( MutableQueryTimeout cancellable, boolean wrapWithExitableDirectoryReader, Executor executor, - boolean reverseLeafReaderContexts + SearchContext searchContext ) throws IOException { super(wrapWithExitableDirectoryReader ? new ExitableDirectoryReader((DirectoryReader) reader, cancellable) : reader, executor); setSimilarity(similarity); setQueryCache(queryCache); setQueryCachingPolicy(queryCachingPolicy); this.cancellable = cancellable; - this.reverseLeafReaderContexts = reverseLeafReaderContexts; + this.searchContext = searchContext; } public void setProfiler(QueryProfiler profiler) { @@ -284,8 +283,10 @@ public void search( @Override protected void search(List leaves, Weight weight, Collector collector) throws IOException { - if (reverseLeafReaderContexts) { + if (shouldReverseLeafReaderContexts()) { // reverse the segment search order if this flag is true. + // Certain queries can benefit if we reverse the segment read order, + // for example time series based queries if searched for desc sort order. for (int i = leaves.size() - 1; i >= 0; i--) { searchLeaf(leaves.get(i), weight, collector); } @@ -303,6 +304,12 @@ protected void search(List leaves, Weight weight, Collector c * the provided ctx. */ private void searchLeaf(LeafReaderContext ctx, Weight weight, Collector collector) throws IOException { + + // Check if at all we need to call this leaf for collecting results. + if (canMatch(ctx) == false) { + return; + } + cancellable.checkCancelled(); weight = wrapWeight(weight); // See please https://github.com/apache/lucene/pull/964 @@ -478,4 +485,43 @@ public void clear() { runnables.clear(); } } + + private boolean canMatch(LeafReaderContext ctx) throws IOException { + // skip segments for search after if min/max of them doesn't qualify competitive + return canMatchSearchAfter(ctx); + } + + private boolean canMatchSearchAfter(LeafReaderContext ctx) throws IOException { + if (searchContext != null && searchContext.request() != null && searchContext.request().source() != null) { + // Only applied on primary sort field and primary search_after. + FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(searchContext.request().source()); + if (primarySortField != null) { + MinAndMax minMax = FieldSortBuilder.getMinMaxOrNullForSegment( + this.searchContext.getQueryShardContext(), + ctx, + primarySortField + ); + return SearchService.canMatchSearchAfter(searchContext.searchAfter(), minMax, primarySortField); + } + } + return true; + } + + private boolean shouldReverseLeafReaderContexts() { + // Time series based workload by default traverses segments in desc order i.e. latest to the oldest order. + // This is actually beneficial for search queries to start search on latest segments first for time series workload. + // That can slow down ASC order queries on timestamp workload. So to avoid that slowdown, we will reverse leaf + // reader order here. + if (searchContext != null && searchContext.indexShard().isTimeSeriesIndex()) { + // Only reverse order for asc order sort queries + if (searchContext.request() != null + && searchContext.request().source() != null + && searchContext.request().source().sorts() != null + && searchContext.request().source().sorts().size() > 0 + && searchContext.request().source().sorts().get(0).order() == SortOrder.ASC) { + return true; + } + } + return false; + } } diff --git a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java index 8f33b51ff392d..0b7b9cd07c300 100644 --- a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java @@ -34,6 +34,7 @@ import org.apache.lucene.document.LongPoint; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.MultiTerms; import org.apache.lucene.index.PointValues; import org.apache.lucene.index.Terms; @@ -605,17 +606,31 @@ public static FieldSortBuilder getPrimaryFieldSortOrNull(SearchSourceBuilder sou } /** - * Return the {@link MinAndMax} indexed value from the provided {@link FieldSortBuilder} or null if unknown. + * Return the {@link MinAndMax} indexed value for shard from the provided {@link FieldSortBuilder} or null if unknown. * The value can be extracted on non-nested indexed mapped fields of type keyword, numeric or date, other fields * and configurations return null. */ public static MinAndMax getMinMaxOrNull(QueryShardContext context, FieldSortBuilder sortBuilder) throws IOException { + return getMinMaxOrNullInternal(context.getIndexReader(), context, sortBuilder); + } + + /** + * Return the {@link MinAndMax} indexed value for segment from the provided {@link FieldSortBuilder} or null if unknown. + * The value can be extracted on non-nested indexed mapped fields of type keyword, numeric or date, other fields + * and configurations return null. + */ + public static MinAndMax getMinMaxOrNullForSegment(QueryShardContext context, LeafReaderContext ctx, FieldSortBuilder sortBuilder) + throws IOException { + return getMinMaxOrNullInternal(ctx.reader(), context, sortBuilder); + } + + private static MinAndMax getMinMaxOrNullInternal(IndexReader reader, QueryShardContext context, FieldSortBuilder sortBuilder) + throws IOException { SortAndFormats sort = SortBuilder.buildSort(Collections.singletonList(sortBuilder), context).get(); SortField sortField = sort.sort.getSort()[0]; if (sortField.getField() == null) { return null; } - IndexReader reader = context.getIndexReader(); MappedFieldType fieldType = context.fieldMapper(sortField.getField()); if (reader == null || (fieldType == null || fieldType.isSearchable() == false)) { return null; diff --git a/server/src/main/java/org/opensearch/search/sort/MinAndMax.java b/server/src/main/java/org/opensearch/search/sort/MinAndMax.java index c5aae37f49052..7e655ca029035 100644 --- a/server/src/main/java/org/opensearch/search/sort/MinAndMax.java +++ b/server/src/main/java/org/opensearch/search/sort/MinAndMax.java @@ -32,12 +32,14 @@ package org.opensearch.search.sort; +import org.apache.lucene.util.BytesRef; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.io.stream.Writeable; import org.opensearch.common.lucene.Lucene; import java.io.IOException; +import java.math.BigInteger; import java.util.Comparator; import java.util.Objects; @@ -93,4 +95,36 @@ public static Comparator> getComparator(SortOrder order) { } return Comparator.nullsLast(cmp); } + + /** + * Compare given object with min + */ + public int compareMin(Object object) { + return compare(getMin(), object); + } + + /** + * Compare given object with max + */ + public int compareMax(Object object) { + return compare(getMax(), object); + } + + private int compare(T one, Object two) { + if (one instanceof Long) { + return Long.compare((Long) one, (Long) two); + } else if (one instanceof Integer) { + return Integer.compare((Integer) one, (Integer) two); + } else if (one instanceof Float) { + return Float.compare((Float) one, (Float) two); + } else if (one instanceof Double) { + return Double.compare((Double) one, (Double) two); + } else if (one instanceof BigInteger) { + return ((BigInteger) one).compareTo((BigInteger) two); + } else if (one instanceof BytesRef) { + return ((BytesRef) one).compareTo((BytesRef) two); + } else { + throw new UnsupportedOperationException("compare type not supported : " + one.getClass()); + } + } } diff --git a/server/src/test/java/org/opensearch/search/SearchServiceTests.java b/server/src/test/java/org/opensearch/search/SearchServiceTests.java index a09e05915b779..a17834bccb238 100644 --- a/server/src/test/java/org/opensearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/opensearch/search/SearchServiceTests.java @@ -35,6 +35,7 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.FilterDirectoryReader; import org.apache.lucene.index.LeafReader; +import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; import org.apache.lucene.store.AlreadyClosedException; import org.opensearch.action.ActionListener; @@ -104,6 +105,9 @@ import org.opensearch.search.internal.ShardSearchContextId; import org.opensearch.search.internal.ShardSearchRequest; import org.opensearch.search.query.QuerySearchResult; +import org.opensearch.search.sort.FieldSortBuilder; +import org.opensearch.search.sort.MinAndMax; +import org.opensearch.search.sort.SortOrder; import org.opensearch.search.suggest.SuggestBuilder; import org.opensearch.test.OpenSearchSingleNodeTestCase; import org.opensearch.threadpool.ThreadPool; @@ -1562,4 +1566,82 @@ public void validatePitStats(String index, long expectedPitCurrent, long expecte assertEquals(expectedPitCurrent, indexShard.searchStats().getTotal().getPitCurrent()); assertEquals(expectedPitCount, indexShard.searchStats().getTotal().getPitCount()); } + + /** + * Test for ASC order search_after query. + * Min = 0L, Max = 9L, search_after = 10L + * Expected result is canMatch = false + */ + public void testCanMatchSearchAfterAscGreaterThanMax() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 10L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + FieldSortBuilder primarySort = new FieldSortBuilder("test"); + primarySort.order(SortOrder.ASC); + assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort), false); + } + + /** + * Test for ASC order search_after query. + * Min = 0L, Max = 9L, search_after = 7L + * Expected result is canMatch = true + */ + public void testCanMatchSearchAfterAscLessThanMax() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 7L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + FieldSortBuilder primarySort = new FieldSortBuilder("test"); + primarySort.order(SortOrder.ASC); + assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort), true); + } + + /** + * Test for ASC order search_after query. + * Min = 0L, Max = 9L, search_after = 9L + * Expected result is canMatch = true + */ + public void testCanMatchSearchAfterAscEqualMax() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 9L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + FieldSortBuilder primarySort = new FieldSortBuilder("test"); + primarySort.order(SortOrder.ASC); + assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort), true); + } + + /** + * Test for DESC order search_after query. + * Min = 0L, Max = 9L, search_after = 10L + * Expected result is canMatch = true + */ + public void testCanMatchSearchAfterDescGreaterThanMin() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 10L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + FieldSortBuilder primarySort = new FieldSortBuilder("test"); + primarySort.order(SortOrder.DESC); + assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort), true); + } + + /** + * Test for DESC order search_after query. + * Min = 0L, Max = 9L, search_after = -1L + * Expected result is canMatch = false + */ + public void testCanMatchSearchAfterDescLessThanMin() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { -1L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + FieldSortBuilder primarySort = new FieldSortBuilder("test"); + primarySort.order(SortOrder.DESC); + assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort), false); + } + + /** + * Test for DESC order search_after query. + * Min = 0L, Max = 9L, search_after = 0L + * Expected result is canMatch = true + */ + public void testCanMatchSearchAfterDescEqualMin() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 0L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + FieldSortBuilder primarySort = new FieldSortBuilder("test"); + primarySort.order(SortOrder.DESC); + assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort), true); + } } diff --git a/server/src/test/java/org/opensearch/search/sort/MinAndMaxTests.java b/server/src/test/java/org/opensearch/search/sort/MinAndMaxTests.java new file mode 100644 index 0000000000000..7557a81db0ad3 --- /dev/null +++ b/server/src/test/java/org/opensearch/search/sort/MinAndMaxTests.java @@ -0,0 +1,61 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.search.sort; + +import org.apache.lucene.util.BytesRef; +import org.opensearch.test.OpenSearchTestCase; + +import java.math.BigInteger; + +public class MinAndMaxTests extends OpenSearchTestCase { + + public void testCompareMin() { + assertEquals(true, new MinAndMax(0L, 9L).compareMin(15L) < 0); // LONG + assertEquals(true, new MinAndMax(0, 9).compareMin(15) < 0); // INT + assertEquals(true, new MinAndMax(0f, 9f).compareMin(15f) < 0); // FLOAT + assertEquals(true, new MinAndMax(0d, 9d).compareMin(15d) < 0); // DOUBLE + assertEquals(true, new MinAndMax(BigInteger.valueOf(0), BigInteger.valueOf(9)).compareMin(BigInteger.valueOf(15)) < 0); // BigInteger + assertEquals(true, new MinAndMax(new BytesRef("a"), new BytesRef("b")).compareMin(new BytesRef("c")) < 0); // ByteRef + expectThrows(UnsupportedOperationException.class, () -> new MinAndMax("a", "b").compareMin("c")); + } + + public void testCompareMax() { + assertEquals(true, new MinAndMax(0L, 9L).compareMax(15L) < 0); // LONG + assertEquals(true, new MinAndMax(0, 9).compareMax(15) < 0); // INT + assertEquals(true, new MinAndMax(0f, 9f).compareMax(15f) < 0); // FLOAT + assertEquals(true, new MinAndMax(0d, 9d).compareMax(15d) < 0); // DOUBLE + assertEquals(true, new MinAndMax(BigInteger.valueOf(0), BigInteger.valueOf(9)).compareMax(BigInteger.valueOf(15)) < 0); // BigInteger + assertEquals(true, new MinAndMax(new BytesRef("a"), new BytesRef("b")).compareMax(new BytesRef("c")) < 0); // ByteRef + expectThrows(UnsupportedOperationException.class, () -> new MinAndMax("a", "b").compareMax("c")); + } +}