Change to determine if concurrent segment search should be used by th…

…e request during SearchContext creation. It caches the e evaluated output for all future invocation. It also provide executor to IndexSearcher based on this evaluation Signed-off-by: Sorabh Hamirwasia <[email protected]>
sohami · Aug 2, 2023 · b6b7722 · b6b7722
1 parent 57eb105
commit b6b7722
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 13 deletions.
diff --git a/server/src/main/java/org/opensearch/search/DefaultSearchContext.java b/server/src/main/java/org/opensearch/search/DefaultSearchContext.java
@@ -183,6 +183,8 @@ final class DefaultSearchContext extends SearchContext {
     private final QueryShardContext queryShardContext;
     private final FetchPhase fetchPhase;
     private final Function<SearchSourceBuilder, InternalAggregation.ReduceContextBuilder> requestToAggReduceContextBuilder;
+    private final Executor concurrentSearchExecutor;
+    private final boolean useConcurrentSearch;
 
     DefaultSearchContext(
         ReaderContext readerContext,
@@ -213,13 +215,15 @@ final class DefaultSearchContext extends SearchContext {
         this.indexShard = readerContext.indexShard();
         this.clusterService = clusterService;
         this.engineSearcher = readerContext.acquireSearcher("search");
+        this.concurrentSearchExecutor = executor;
+        this.useConcurrentSearch = useConcurrentSearch();
         this.searcher = new ContextIndexSearcher(
             engineSearcher.getIndexReader(),
             engineSearcher.getSimilarity(),
             engineSearcher.getQueryCache(),
             engineSearcher.getQueryCachingPolicy(),
             lowLevelCancellation,
-            executor,
+            useConcurrentSearch ? executor : null,
             this
         );
         this.relativeTimeSupplier = relativeTimeSupplier;
@@ -878,18 +882,7 @@ public Profilers getProfilers() {
      */
     @Override
     public boolean isConcurrentSegmentSearchEnabled() {
-        if (FeatureFlags.isEnabled(FeatureFlags.CONCURRENT_SEGMENT_SEARCH)
-            && (clusterService != null)
-            && (searcher().getExecutor() != null)) {
-            return indexService.getIndexSettings()
-                .getSettings()
-                .getAsBoolean(
-                    IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(),
-                    clusterService.getClusterSettings().get(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING)
-                );
-        } else {
-            return false;
-        }
+        return useConcurrentSearch;
     }
 
     public void setProfilers(Profilers profilers) {
@@ -932,4 +925,24 @@ public void setBucketCollectorProcessor(BucketCollectorProcessor bucketCollector
     public BucketCollectorProcessor bucketCollectorProcessor() {
         return bucketCollectorProcessor;
     }
+
+    /**
+     * Evaluate based on cluster and index settings if concurrent segment search should be used for this request context
+     * @return true: use concurrent search <br/>
+     *         false: otherwise
+     */
+    private boolean useConcurrentSearch() {
+        if (FeatureFlags.isEnabled(FeatureFlags.CONCURRENT_SEGMENT_SEARCH)
+            && (clusterService != null)
+            && (concurrentSearchExecutor != null)) {
+            return indexService.getIndexSettings()
+                .getSettings()
+                .getAsBoolean(
+                    IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(),
+                    clusterService.getClusterSettings().get(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING)
+                );
+        } else {
+            return false;
+        }
+    }
 }
diff --git a/server/src/main/java/org/opensearch/search/internal/FilteredSearchContext.java b/server/src/main/java/org/opensearch/search/internal/FilteredSearchContext.java
@@ -559,4 +559,9 @@ public void setBucketCollectorProcessor(BucketCollectorProcessor bucketCollector
     public BucketCollectorProcessor bucketCollectorProcessor() {
         return in.bucketCollectorProcessor();
     }
+
+    @Override
+    public boolean isConcurrentSegmentSearchEnabled() {
+        return in.isConcurrentSegmentSearchEnabled();
+    }
 }
diff --git a/server/src/test/java/org/opensearch/search/SearchServiceTests.java b/server/src/test/java/org/opensearch/search/SearchServiceTests.java
@@ -1270,6 +1270,12 @@ public void testConcurrentSegmentSearchSearchContext() throws IOException {
                         .getSetting(index, IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey())
                 );
                 assertEquals(concurrentSearchEnabled, searchContext.isConcurrentSegmentSearchEnabled());
+                // verify executor nullability with concurrent search enabled/disabled
+                if (concurrentSearchEnabled) {
+                    assertNotNull(searchContext.searcher().getExecutor());
+                } else {
+                    assertNull(searchContext.searcher().getExecutor());
+                }
             }
         }
         // Cleanup