diff --git a/server/src/internalClusterTest/java/org/elasticsearch/action/search/TransportSearchIT.java b/server/src/internalClusterTest/java/org/elasticsearch/action/search/TransportSearchIT.java index 725b6199918cc..68b3d11a24ab0 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/action/search/TransportSearchIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/action/search/TransportSearchIT.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.search; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; @@ -578,7 +577,7 @@ public void close() {} @Override public LeafBucketCollector getLeafCollector(LeafReaderContext ctx) throws IOException { - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java b/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java index ef14be9ab42c2..521b6a80a46d3 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java @@ -44,7 +44,7 @@ public void preProcess(SearchContext context) { } context.aggregations().aggregators(aggregators); if (collectors.isEmpty() == false) { - Collector collector = MultiBucketCollector.wrap(collectors); + Collector collector = MultiBucketCollector.wrap(true, collectors); ((BucketCollector)collector).preCollection(); if (context.getProfilers() != null) { collector = new InternalProfileCollector(collector, CollectorResult.REASON_AGGREGATION, @@ -80,7 +80,7 @@ public void execute(SearchContext context) { // optimize the global collector based execution if (globals.isEmpty() == false) { - BucketCollector globalsCollector = MultiBucketCollector.wrap(globals); + BucketCollector globalsCollector = MultiBucketCollector.wrap(false, globals); Query query = context.buildFilteredQuery(Queries.newMatchAllQuery()); try { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java b/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java index bffd626a13cbc..fd0d6d06209c8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java @@ -156,6 +156,12 @@ public Map metadata() { /** * Get a {@link LeafBucketCollector} for the given ctx, which should * delegate to the given collector. + *

+ * {@linkplain Aggregator}s that perform collection independent of the main + * search should collect the provided leaf in their implementation of this + * method and return {@link LeafBucketCollector#NO_OP_COLLECTOR} to signal + * that they don't need to be collected with the main search. We'll remove + * them from the list of collectors. */ protected abstract LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucketCollector sub) throws IOException; @@ -181,8 +187,7 @@ protected void doPreCollection() throws IOException { @Override public final void preCollection() throws IOException { - List collectors = Arrays.asList(subAggregators); - collectableSubAggregators = MultiBucketCollector.wrap(collectors); + collectableSubAggregators = MultiBucketCollector.wrap(false, Arrays.asList(subAggregators)); doPreCollection(); collectableSubAggregators.preCollection(); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketCollector.java b/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketCollector.java index 0c2d2f54531a4..e5a357938046d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketCollector.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketCollector.java @@ -29,24 +29,26 @@ * the null ones. */ public class MultiBucketCollector extends BucketCollector { - - /** See {@link #wrap(Iterable)}. */ - public static BucketCollector wrap(BucketCollector... collectors) { - return wrap(Arrays.asList(collectors)); - } - /** * Wraps a list of {@link BucketCollector}s with a {@link MultiBucketCollector}. This * method works as follows: *

+ * @param terminateIfNoop Pass true if {@link #getLeafCollector} should throw + * {@link CollectionTerminatedException} if all leaf collectors are noop. Pass + * false if terminating would break stuff. The top level collection for + * aggregations should pass true here because we want to skip collections if + * all aggregations return NOOP. But when aggregtors themselves call this + * method they chould *generally* pass false here because they have collection + * actions to perform even if their sub-aggregators are NOOPs. */ - public static BucketCollector wrap(Iterable collectors) { + public static BucketCollector wrap(boolean terminateIfNoop, Iterable collectors) { // For the user's convenience, we allow NO_OP collectors to be passed. // However, to improve performance, these null collectors are found // and dropped from the array we save for actual collection time. @@ -68,7 +70,43 @@ public static BucketCollector wrap(Iterable collector break; } } - return col; + final BucketCollector collector = col; + // Wrap the collector in one that takes terminateIfNoop into account. + return new BucketCollector() { + @Override + public ScoreMode scoreMode() { + return collector.scoreMode(); + } + + @Override + public void preCollection() throws IOException { + collector.preCollection(); + } + + @Override + public void postCollection() throws IOException { + collector.postCollection(); + } + + @Override + public LeafBucketCollector getLeafCollector(LeafReaderContext ctx) throws IOException { + try { + LeafBucketCollector leafCollector = collector.getLeafCollector(ctx); + if (false == leafCollector.isNoop()) { + return leafCollector; + } + } catch (CollectionTerminatedException e) { + throw new IllegalStateException( + "getLeafCollector should return a noop collector instead of throw " + + CollectionTerminatedException.class.getSimpleName(), e + ); + } + if (terminateIfNoop) { + throw new CollectionTerminatedException(); + } + return LeafBucketCollector.NO_OP_COLLECTOR; + } + }; } else { BucketCollector[] colls = new BucketCollector[n]; n = 0; @@ -77,14 +115,16 @@ public static BucketCollector wrap(Iterable collector colls[n++] = c; } } - return new MultiBucketCollector(colls); + return new MultiBucketCollector(terminateIfNoop, colls); } } + private final boolean terminateIfNoop; private final boolean cacheScores; private final BucketCollector[] collectors; - private MultiBucketCollector(BucketCollector... collectors) { + private MultiBucketCollector(boolean terminateIfNoop, BucketCollector... collectors) { + this.terminateIfNoop = terminateIfNoop; this.collectors = collectors; int numNeedsScores = 0; for (Collector collector : collectors) { @@ -129,26 +169,27 @@ public String toString() { @Override public LeafBucketCollector getLeafCollector(LeafReaderContext context) throws IOException { - final List leafCollectors = new ArrayList<>(); + final List leafCollectors = new ArrayList<>(collectors.length); for (BucketCollector collector : collectors) { - final LeafBucketCollector leafCollector; try { - leafCollector = collector.getLeafCollector(context); + LeafBucketCollector leafCollector = collector.getLeafCollector(context); + if (false == leafCollector.isNoop()) { + leafCollectors.add(leafCollector); + } } catch (CollectionTerminatedException e) { - // this leaf collector does not need this segment - continue; + throw new IllegalStateException( + "getLeafCollector should return a noop collector instead of throw " + + CollectionTerminatedException.class.getSimpleName(), + e + ); } - leafCollectors.add(leafCollector); } switch (leafCollectors.size()) { case 0: - // TODO it's probably safer to return noop and let the caller throw if it wants to - /* - * See MinAggregator which only throws if it has a parent. - * That is because it doesn't want there to ever drop - * to this case and throw, thus skipping calculating the parent. - */ - throw new CollectionTerminatedException(); + if (terminateIfNoop) { + throw new CollectionTerminatedException(); + } + return LeafBucketCollector.NO_OP_COLLECTOR; case 1: return leafCollectors.get(0); default: diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/BestBucketsDeferringCollector.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/BestBucketsDeferringCollector.java index 008b877809c52..385c2a5b9cc41 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/BestBucketsDeferringCollector.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/BestBucketsDeferringCollector.java @@ -85,7 +85,7 @@ public ScoreMode scoreMode() { /** Set the deferred collectors. */ @Override public void setDeferredCollector(Iterable deferredCollectors) { - this.collector = MultiBucketCollector.wrap(deferredCollectors); + this.collector = MultiBucketCollector.wrap(true, deferredCollectors); } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferableBucketAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferableBucketAggregator.java index 3c2658b646614..3e18498317a78 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferableBucketAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/DeferableBucketAggregator.java @@ -57,7 +57,7 @@ protected void doPreCollection() throws IOException { deferringCollector.setDeferredCollector(deferredAggregations); collectors.add(deferringCollector); } - collectableSubAggregators = MultiBucketCollector.wrap(collectors); + collectableSubAggregators = MultiBucketCollector.wrap(false, collectors); } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java index d71e60c3c5569..ca6ec86b6c41f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java @@ -118,8 +118,7 @@ protected void doClose() { @Override protected void doPreCollection() throws IOException { - List collectors = Arrays.asList(subAggregators); - deferredCollectors = MultiBucketCollector.wrap(collectors); + deferredCollectors = MultiBucketCollector.wrap(false, Arrays.asList(subAggregators)); collectableSubAggregators = BucketCollector.NO_OP_COLLECTOR; } @@ -388,7 +387,7 @@ protected LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucket // Throwing this exception will terminate the execution of the search for this root aggregation, // see {@link MultiCollector} for more details on how we handle early termination in aggregations. earlyTerminated = true; - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } else { if (fillDocIdSet) { currentLeaf = ctx; @@ -399,7 +398,7 @@ protected LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucket // that is after the index sort prefix using the rawAfterKey and we start collecting // document from there. processLeafFromQuery(ctx, indexSortPrefix); - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } else { final LeafBucketCollector inner = queue.getLeafCollector(ctx, getFirstPassCollector(docIdSetBuilder, sortPrefixLen)); return new LeafBucketCollector() { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregator.java index 1c9e591aa40dc..53070495695df 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregator.java @@ -9,7 +9,6 @@ package org.elasticsearch.search.aggregations.bucket.filter; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.LeafCollector; import org.apache.lucene.search.Scorable; import org.apache.lucene.util.Bits; @@ -394,8 +393,7 @@ protected LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucket } else { collectSubs(ctx, live, sub); } - // Throwing this exception is how we communicate to the collection mechanism that we don't need the segment. - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/BestDocsDeferringCollector.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/BestDocsDeferringCollector.java index e83d1054ae67f..77208ee5bb7f8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/BestDocsDeferringCollector.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/BestDocsDeferringCollector.java @@ -73,7 +73,7 @@ public ScoreMode scoreMode() { /** Set the deferred collectors. */ @Override public void setDeferredCollector(Iterable deferredCollectors) { - this.deferred = MultiBucketCollector.wrap(deferredCollectors); + this.deferred = MultiBucketCollector.wrap(true, deferredCollectors); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregator.java index 36f0df6c23441..88a7ebf9c3a81 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregator.java @@ -10,7 +10,6 @@ import org.apache.lucene.index.LeafReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.PointValues; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.apache.lucene.util.Bits; import org.apache.lucene.util.FutureArrays; @@ -71,12 +70,7 @@ public ScoreMode scoreMode() { public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { if (valuesSource == null) { - if (parent != null) { - return LeafBucketCollector.NO_OP_COLLECTOR; - } else { - // we have no parent and the values source is empty so we can skip collecting hits. - throw new CollectionTerminatedException(); - } + return LeafBucketCollector.NO_OP_COLLECTOR; } if (pointConverter != null) { Number segMax = findLeafMaxValue(ctx.reader(), pointField, pointConverter); @@ -90,7 +84,7 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, max = Math.max(max, segMax.doubleValue()); maxes.set(0, max); // the maximum value has been extracted, we don't need to collect hits on this segment. - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } } final SortedNumericDoubleValues allValues = valuesSource.doubleValues(ctx); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java index 0bafbedf75de4..dd26f9408dfff 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java @@ -72,12 +72,7 @@ public ScoreMode scoreMode() { public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { if (valuesSource == null) { - if (parent == null) { - return LeafBucketCollector.NO_OP_COLLECTOR; - } else { - // we have no parent and the values source is empty so we can skip collecting hits. - throw new CollectionTerminatedException(); - } + return LeafBucketCollector.NO_OP_COLLECTOR; } if (pointConverter != null) { Number segMin = findLeafMinValue(ctx.reader(), pointField, pointConverter); @@ -90,13 +85,12 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, min = Math.min(min, segMin.doubleValue()); mins.set(0, min); // the minimum value has been extracted, we don't need to collect hits on this segment. - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } } final SortedNumericDoubleValues allValues = valuesSource.doubleValues(ctx); final NumericDoubleValues values = MultiValueMode.MIN.select(allValues); return new LeafBucketCollectorBase(sub, allValues) { - @Override public void collect(int doc, long bucket) throws IOException { if (bucket >= mins.size()) { diff --git a/server/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingLeafBucketCollector.java b/server/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingLeafBucketCollector.java index 1b2ae5e911ccb..1e8d8ee1cd5d9 100644 --- a/server/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingLeafBucketCollector.java +++ b/server/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingLeafBucketCollector.java @@ -39,4 +39,9 @@ public void setScorer(Scorable scorer) throws IOException { delegate.setScorer(scorer); } + @Override + public boolean isNoop() { + return delegate.isNoop(); + } + } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/MultiBucketCollectorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/MultiBucketCollectorTests.java index 05cd13918e792..d6b0d11ff15c0 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/MultiBucketCollectorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/MultiBucketCollectorTests.java @@ -29,6 +29,8 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import static org.hamcrest.Matchers.equalTo; + public class MultiBucketCollectorTests extends ESTestCase { private static class ScoreAndDoc extends Scorable { float score; @@ -59,7 +61,7 @@ private static class TerminateAfterBucketCollector extends BucketCollector { @Override public LeafBucketCollector getLeafCollector(LeafReaderContext context) throws IOException { if (count >= terminateAfter) { - throw new CollectionTerminatedException(); + return LeafBucketCollector.NO_OP_COLLECTOR; } final LeafBucketCollector leafCollector = in.getLeafCollector(context); return new LeafBucketCollectorBase(leafCollector, null) { @@ -175,7 +177,7 @@ public void testCollectionTerminatedExceptionHandling() throws IOException { expectedCounts.put(collector, expectedCount); collectors.add(new TerminateAfterBucketCollector(collector, terminateAfter)); } - searcher.search(new MatchAllDocsQuery(), MultiBucketCollector.wrap(collectors)); + searcher.search(new MatchAllDocsQuery(), MultiBucketCollector.wrap(true, collectors)); for (Map.Entry expectedCount : expectedCounts.entrySet()) { assertEquals(expectedCount.getValue().intValue(), expectedCount.getKey().getTotalHits()); } @@ -184,6 +186,54 @@ public void testCollectionTerminatedExceptionHandling() throws IOException { } } + public void testNotTerminated() throws IOException { + final int iters = atLeast(3); + for (int iter = 0; iter < iters; ++iter) { + try (Directory dir = newDirectory()) { + RandomIndexWriter w = new RandomIndexWriter(random(), dir); + final int numDocs = randomIntBetween(100, 1000); + final Document doc = new Document(); + for (int i = 0; i < numDocs; ++i) { + w.addDocument(doc); + } + try (IndexReader reader = w.getReader()) { + w.close(); + Map expectedCounts = new HashMap<>(); + List collectors = new ArrayList<>(); + final int numCollectors = randomIntBetween(1, 5); + for (int i = 0; i < numCollectors; ++i) { + final int terminateAfter = random().nextInt(numDocs + 10); + final int expectedCount = terminateAfter > numDocs ? numDocs : terminateAfter; + TotalHitCountBucketCollector collector = new TotalHitCountBucketCollector(); + expectedCounts.put(collector, expectedCount); + collectors.add(new TerminateAfterBucketCollector(collector, terminateAfter)); + } + BucketCollector wrapped = MultiBucketCollector.wrap(false, collectors); + for (LeafReaderContext ctx : reader.leaves()) { + boolean shouldNoop = true; + for (Map.Entry expectedCount : expectedCounts.entrySet()) { + shouldNoop &= expectedCount.getValue().intValue() <= expectedCount.getKey().getTotalHits(); + } + LeafBucketCollector collector = wrapped.getLeafCollector(ctx); + assertThat(collector.isNoop(), equalTo(shouldNoop)); + if (false == collector.isNoop()) { + for (int docId = 0; docId < ctx.reader().numDocs(); docId++) { + try { + collector.collect(docId); + } catch (CollectionTerminatedException e) { + break; + } + } + } + } + for (Map.Entry expectedCount : expectedCounts.entrySet()) { + assertEquals(expectedCount.getValue().intValue(), expectedCount.getKey().getTotalHits()); + } + } + } + } + } + public void testSetScorerAfterCollectionTerminated() throws IOException { BucketCollector collector1 = new TotalHitCountBucketCollector(); BucketCollector collector2 = new TotalHitCountBucketCollector(); @@ -201,7 +251,7 @@ public void testSetScorerAfterCollectionTerminated() throws IOException { List collectors = Arrays.asList(collector1, collector2); Collections.shuffle(collectors, random()); - BucketCollector collector = MultiBucketCollector.wrap(collectors); + BucketCollector collector = MultiBucketCollector.wrap(true, collectors); LeafBucketCollector leafCollector = collector.getLeafCollector(null); leafCollector.setScorer(scorer); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java index 6260182df1493..bb78f5e496133 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java @@ -16,7 +16,6 @@ import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.RandomIndexWriter; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; @@ -52,6 +51,7 @@ import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.InternalAggregation; +import org.elasticsearch.search.aggregations.LeafBucketCollector; import org.elasticsearch.search.aggregations.bucket.filter.FiltersAggregator.KeyedFilter; import org.elasticsearch.search.aggregations.bucket.nested.NestedAggregatorTests; import org.elasticsearch.search.aggregations.metrics.InternalMax; @@ -788,8 +788,6 @@ public void testSubAggsManyFilters() throws IOException { }, dateFt, intFt); } - - @Override protected List objectMappers() { return MOCK_OBJECT_MAPPERS; @@ -798,7 +796,8 @@ protected List objectMappers() { private Map collectAndGetFilterDebugInfo(IndexSearcher searcher, Aggregator aggregator) throws IOException { aggregator.preCollection(); for (LeafReaderContext ctx : searcher.getIndexReader().leaves()) { - expectThrows(CollectionTerminatedException.class, () -> aggregator.getLeafCollector(ctx)); + LeafBucketCollector leafCollector = aggregator.getLeafCollector(ctx); + assertTrue(leafCollector.isNoop()); } Map debug = new HashMap<>(); ((FiltersAggregator.FilterByFilter) aggregator).filters().get(0).collectDebugInfo(debug::put); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index e0f6182f308a3..fee43c03b6b51 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -799,7 +799,10 @@ public void testEarlyTermination() throws Exception { MaxAggregator maxAggregator = createAggregator(maxAggregationBuilder, indexSearcher, fieldType); ValueCountAggregator countAggregator = createAggregator(countAggregationBuilder, indexSearcher, fieldType); - BucketCollector bucketCollector = MultiBucketCollector.wrap(maxAggregator, countAggregator); + BucketCollector bucketCollector = MultiBucketCollector.wrap( + true, + org.elasticsearch.common.collect.List.of(maxAggregator, countAggregator) + ); bucketCollector.preCollection(); indexSearcher.search(new MatchAllDocsQuery(), bucketCollector); bucketCollector.postCollection(); @@ -851,7 +854,10 @@ public void testNestedEarlyTermination() throws Exception { ValueCountAggregator countAggregator = createAggregator(countAggregationBuilder, indexSearcher, multiValuesfieldType); TermsAggregator termsAggregator = createAggregator(termsAggregationBuilder, indexSearcher, singleValueFieldType); - BucketCollector bucketCollector = MultiBucketCollector.wrap(maxAggregator, countAggregator, termsAggregator); + BucketCollector bucketCollector = MultiBucketCollector.wrap( + true, + org.elasticsearch.common.collect.List.of(maxAggregator, countAggregator, termsAggregator) + ); bucketCollector.preCollection(); indexSearcher.search(new MatchAllDocsQuery(), bucketCollector); bucketCollector.postCollection(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java index 52579fed223d5..724afff09d441 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java @@ -85,8 +85,8 @@ public class MinAggregatorTests extends AggregatorTestCase { - private final String SCRIPT_NAME = "script_name"; - private final long SCRIPT_VALUE = 19L; + private static final String SCRIPT_NAME = "script_name"; + private static final long SCRIPT_VALUE = 19L; /** Script to take a field name in params and sum the values of the field. */ private static final String SUM_FIELD_PARAMS_SCRIPT = "sum_field_params"; diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregator.java index 09528d398aa05..7e1cd57145792 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregator.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.analytics.aggregations.metrics; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.util.DoubleArray; @@ -57,12 +56,7 @@ public ScoreMode scoreMode() { @Override public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { if (valuesSource == null) { - if (parent != null) { - return LeafBucketCollector.NO_OP_COLLECTOR; - } else { - // we have no parent and the values source is empty so we can skip collecting hits. - throw new CollectionTerminatedException(); - } + return LeafBucketCollector.NO_OP_COLLECTOR; } final HistogramValues values = valuesSource.getHistogramValues(ctx); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregator.java index 8bb4f7a7ecf2f..3b1de6d2af504 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregator.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.analytics.aggregations.metrics; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.util.DoubleArray; @@ -57,12 +56,7 @@ public ScoreMode scoreMode() { @Override public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { if (valuesSource == null) { - if (parent == null) { - return LeafBucketCollector.NO_OP_COLLECTOR; - } else { - // we have no parent and the values source is empty so we can skip collecting hits. - throw new CollectionTerminatedException(); - } + return LeafBucketCollector.NO_OP_COLLECTOR; } final HistogramValues values = valuesSource.getHistogramValues(ctx); diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregator.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregator.java index 2e0f6f17f8645..0c4e9b104d882 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregator.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregator.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.aggregatemetric.aggregations.metrics; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.util.BigArrays; @@ -60,12 +59,7 @@ public ScoreMode scoreMode() { @Override public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { if (valuesSource == null) { - if (parent != null) { - return LeafBucketCollector.NO_OP_COLLECTOR; - } else { - // we have no parent and the values source is empty so we can skip collecting hits. - throw new CollectionTerminatedException(); - } + return LeafBucketCollector.NO_OP_COLLECTOR; } final BigArrays bigArrays = bigArrays(); diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregator.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregator.java index c50363079ccc2..a098439983a05 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregator.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregator.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.aggregatemetric.aggregations.metrics; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.util.BigArrays; @@ -60,12 +59,7 @@ public ScoreMode scoreMode() { @Override public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { if (valuesSource == null) { - if (parent == null) { - return LeafBucketCollector.NO_OP_COLLECTOR; - } else { - // we have no parent and the values source is empty so we can skip collecting hits. - throw new CollectionTerminatedException(); - } + return LeafBucketCollector.NO_OP_COLLECTOR; } final BigArrays bigArrays = bigArrays();