From 3e91183cc6bfce416fb98ee21616b5d0c9d9a309 Mon Sep 17 00:00:00 2001 From: Jay Deng Date: Tue, 25 Jul 2023 11:02:32 -0700 Subject: [PATCH] Create separate SourceLookup instance per segment slice in SignificantTextAggregatorFactory (#8807) * Remove flakey assertion in SearchTimeoutIT Signed-off-by: Jay Deng * Create separate SourceLookup instance per segment slice in SignificantTextAggregatorFactory Signed-off-by: Jay Deng --------- Signed-off-by: Jay Deng --- CHANGELOG.md | 1 + .../java/org/opensearch/search/SearchTimeoutIT.java | 1 - .../bucket/terms/SignificantTextAggregatorFactory.java | 6 +++--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d219b14722b5d..b2bd4bcd25785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,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 index c7392b260319a..aa8ef3f29c989 100644 --- 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/server/src/main/java/org/opensearch/search/aggregations/bucket/terms/SignificantTextAggregatorFactory.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/terms/SignificantTextAggregatorFactory.java index e5cc3f9dbaabd..7f5804c8b9561 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/terms/SignificantTextAggregatorFactory.java +++ b/server/src/main/java/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 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;