apply review comments

Signed-off-by: panguixin <[email protected]>
opensearch-project · Aug 6, 2024 · 8c91a12 · 8c91a12
1 parent 27a1324
commit 8c91a12
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add ThreadContextPermission for stashAndMergeHeaders and stashWithOrigin ([#15039](https://github.com/opensearch-project/OpenSearch/pull/15039))
 - [Concurrent Segment Search] Support composite aggregations with scripting ([#15072](https://github.com/opensearch-project/OpenSearch/pull/15072))
 - Add `rangeQuery` and `regexpQuery` for `constant_keyword` field type ([#14711](https://github.com/opensearch-project/OpenSearch/pull/14711))
+- Optimize the canMatch phase on the data node ([#14511](https://github.com/opensearch-project/OpenSearch/pull/14511))
 
 ### Dependencies
 - Bump `netty` from 4.1.111.Final to 4.1.112.Final ([#15081](https://github.com/opensearch-project/OpenSearch/pull/15081))

diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java
@@ -1620,29 +1620,27 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre
                 );
                 Rewriteable.rewrite(request.getRewriteable(), context, false);
 
-                if (hasRefreshPending) {
-                    final FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source());
-                    final MinAndMax<?> minMax = sortBuilder != null ? FieldSortBuilder.getMinMaxOrNull(context, sortBuilder) : null;
-                    return new CanMatchResponse(true, minMax);
+                if (hasRefreshPending == false) {
+                    final boolean aliasFilterCannotMatch = request.getAliasFilter().getQueryBuilder() instanceof MatchNoneQueryBuilder;
+                    if (aliasFilterCannotMatch
+                        || (canRewriteToMatchNone(request.source()) && request.source().query() instanceof MatchNoneQueryBuilder)) {
+                        return new CanMatchResponse(false, null);
+                    }
                 }
 
-                final boolean aliasFilterCanMatch = request.getAliasFilter().getQueryBuilder() instanceof MatchNoneQueryBuilder == false;
-                if (aliasFilterCanMatch == false
-                    || (canRewriteToMatchNone(request.source()) && request.source().query() instanceof MatchNoneQueryBuilder)) {
-                    return new CanMatchResponse(false, null);
+                final FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source());
+                final MinAndMax<?> minMax = sortBuilder != null ? FieldSortBuilder.getMinMaxOrNull(context, sortBuilder) : null;
+                if (hasRefreshPending || minMax == null) {
+                    return new CanMatchResponse(true, minMax);
                 }
 
                 boolean canMatch = true;
-                final FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source());
-                final MinAndMax<?> minMax = sortBuilder != null ? FieldSortBuilder.getMinMaxOrNull(context, sortBuilder) : null;
-                final Integer trackTotalHitsUpto = request.source() == null ? null : request.source().trackTotalHitsUpTo();
                 // Skipping search on shard/segment entirely can cause mismatch on total_tracking_hits, hence skip only if
                 // track_total_hits is false.
                 // Check for sort.missing == null, since in case of missing values sort queries, if segment/shard's min/max
                 // is out of search_after range, it still should be printed and hence we should not skip segment/shard.
-                if (Objects.equals(trackTotalHitsUpto, SearchContext.TRACK_TOTAL_HITS_DISABLED)
-                    && minMax != null
-                    && sortBuilder.missing() == null) {
+                final Integer trackTotalHitsUpto = request.source() == null ? null : request.source().trackTotalHitsUpTo();
+                if (Objects.equals(trackTotalHitsUpto, SearchContext.TRACK_TOTAL_HITS_DISABLED) && sortBuilder.missing() == null) {
                     final Object primarySearchAfterField = SearchAfterBuilder.getPrimarySearchAfterFieldOrNull(request.source());
                     if (primarySearchAfterField != null) {
                         final FieldDoc searchAfterFieldDoc = getPrimarySearchAfterFieldDoc(sortBuilder, primarySearchAfterField, context);

diff --git a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java
@@ -518,7 +518,7 @@ private boolean canMatchSearchAfter(LeafReaderContext ctx) throws IOException {
         if (searchContext.searchAfter() != null
             && searchContext.request() != null
             && searchContext.request().source() != null
-            && Objects.equals(searchContext.trackTotalHitsUpTo(), SearchContext.TRACK_TOTAL_HITS_DISABLED)) {
+            && searchContext.trackTotalHitsUpTo() == SearchContext.TRACK_TOTAL_HITS_DISABLED) {
             // Only applied on primary sort field and primary search_after.
             FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(searchContext.request().source());
             // Check for sort.missing == null, since in case of missing values sort queries, if segment/shard's min/max