Create separate SourceLookup instance per segment slice in Significan…

…tTextAggregatorFactory (opensearch-project#8807) * Remove flakey assertion in SearchTimeoutIT Signed-off-by: Jay Deng <[email protected]> * Create separate SourceLookup instance per segment slice in SignificantTextAggregatorFactory Signed-off-by: Jay Deng <[email protected]> --------- Signed-off-by: Jay Deng <[email protected]>
reta · Jul 25, 2023 · b9b5e5c · b9b5e5c
1 parent f607870
commit b9b5e5c
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -94,6 +94,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Change InternalSignificantTerms to sum shard-level superset counts only in final reduce ([#8735](https://github.com/opensearch-project/OpenSearch/pull/8735))
 - Exclude 'benchmarks' from codecov report ([#8805](https://github.com/opensearch-project/OpenSearch/pull/8805))
 - [Refactor] MediaTypeParser to MediaTypeParserRegistry ([#8636](https://github.com/opensearch-project/OpenSearch/pull/8636))
+- Create separate SourceLookup instance per segment slice in SignificantTextAggregatorFactory ([#8807](https://github.com/opensearch-project/OpenSearch/pull/8807))
 
 ### Deprecated
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java
@@ -79,7 +79,6 @@ public void testSimpleTimeout() throws Exception {
             .get();
         assertTrue(searchResponse.isTimedOut());
         assertEquals(0, searchResponse.getFailedShards());
-        assertTrue(numDocs > searchResponse.getHits().getTotalHits().value);
     }
 
     public void testSimpleDoesNotTimeout() throws Exception {

diff --git a/...ava/org/opensearch/search/aggregations/bucket/terms/SignificantTextAggregatorFactory.java b/...ava/org/opensearch/search/aggregations/bucket/terms/SignificantTextAggregatorFactory.java
@@ -148,7 +148,6 @@ protected Aggregator createInternal(
             : includeExclude.convertToStringFilter(DocValueFormat.RAW, maxRegexLength);
 
         MapStringTermsAggregator.CollectorSource collectorSource = new SignificantTextCollectorSource(
-            queryShardContext.lookup().source(),
             queryShardContext.bigArrays(),
             fieldType,
             sourceFieldNames,
@@ -186,13 +185,14 @@ private static class SignificantTextCollectorSource implements MapStringTermsAgg
         private ObjectArray<DuplicateByteSequenceSpotter> dupSequenceSpotters;
 
         SignificantTextCollectorSource(
-            SourceLookup sourceLookup,
             BigArrays bigArrays,
             MappedFieldType fieldType,
             String[] sourceFieldNames,
             boolean filterDuplicateText
         ) {
-            this.sourceLookup = sourceLookup;
+            // Create a new SourceLookup instance per aggregator instead of use the shared one from SearchLookup. This is fine because it
+            // will only be accessed by this Aggregator instance and not anywhere else.
+            this.sourceLookup = new SourceLookup();
             this.bigArrays = bigArrays;
             this.fieldType = fieldType;
             this.sourceFieldNames = sourceFieldNames;