diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilder.java index 99c27130014ea..1632d06d9a1ea 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilder.java @@ -220,17 +220,35 @@ public boolean isInSortOrderExecutionRequired() { } /** - * Return false if this aggregation or any of the child aggregations does not support concurrent search + * Return false if this aggregation or any of the child aggregations does not support concurrent search. + * As a result, such aggregation will always be executed sequentially despite concurrency is enabled for the query phase. + * Note: aggregations that don't support concurrency, may or may not support offloading their collection to the search worker threads, + * depending on what {@link #supportsOffloadingSequentialCollection()} returns. */ public boolean supportsConcurrentExecution() { for (AggregationBuilder builder : factoriesBuilder.getAggregatorFactories()) { - if (builder.supportsConcurrentExecution() == false) { + if (builder.supportsOffloadingSequentialCollection() || builder.supportsConcurrentExecution() == false) { return false; } } return isInSortOrderExecutionRequired() == false; } + /** + * Returns false if this aggregation or any of its child aggregations does not support offloading its sequential collection + * to a separate thread. As a result, such aggregation will always be executed sequentially, and fully in the search thread, + * without offloading its collection to the search worker threads. + * Note: aggregations that don't support offloading sequential collection, don't support concurrency by definition. + */ + public boolean supportsOffloadingSequentialCollection() { + for (AggregationBuilder builder : factoriesBuilder.getAggregatorFactories()) { + if (builder.supportsOffloadingSequentialCollection() == false) { + return false; + } + } + return true; + } + /** * Called by aggregations whose parents must be sequentially ordered. * @param type the type of the aggregation being validated diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java index 92888cb934234..c6092dc0328d8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java @@ -97,7 +97,7 @@ public boolean supportsSampling() { } @Override - public boolean supportsConcurrentExecution() { + public boolean supportsOffloadingSequentialCollection() { return false; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregationBuilder.java index ee34922346b04..d81209844da40 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregationBuilder.java @@ -81,7 +81,7 @@ public BucketCardinality bucketCardinality() { } @Override - public boolean supportsConcurrentExecution() { + public boolean supportsOffloadingSequentialCollection() { return false; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregationBuilder.java index 12bbbac5c36a8..9f857305a7196 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregationBuilder.java @@ -113,7 +113,7 @@ public boolean supportsSampling() { } @Override - public boolean supportsConcurrentExecution() { + public boolean supportsOffloadingSequentialCollection() { return false; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregatorTests.java index 9437b9166d3b4..3d02aa948d635 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregatorTests.java @@ -89,8 +89,6 @@ public class CardinalityAggregatorTests extends AggregatorTestCase { /** Script to extract a collection of numeric values from the 'numbers' field **/ public static final String NUMERIC_VALUES_SCRIPT = "doc['numbers']"; - public static final int HASHER_DEFAULT_SEED = 17; - @Override protected ScriptService getMockScriptService() { final Map, Object>> scripts = new HashMap<>(); diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/BaseAggregationTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/BaseAggregationTestCase.java index eed1e4a921555..c325282fa60ad 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/BaseAggregationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/BaseAggregationTestCase.java @@ -61,10 +61,25 @@ public void testFromXContent() throws IOException { public void testSupportsConcurrentExecution() { AB builder = createTestAggregatorBuilder(); boolean supportsConcurrency = builder.supportsConcurrentExecution(); + if (supportsConcurrency == false) { + assertFalse(builder.supportsOffloadingSequentialCollection()); + } AggregationBuilder bucketBuilder = new HistogramAggregationBuilder("test"); - assertThat(bucketBuilder.supportsConcurrentExecution(), equalTo(true)); + assertTrue(bucketBuilder.supportsConcurrentExecution()); bucketBuilder.subAggregation(builder); assertThat(bucketBuilder.supportsConcurrentExecution(), equalTo(supportsConcurrency)); + if (bucketBuilder.supportsConcurrentExecution() == false) { + assertFalse(bucketBuilder.supportsOffloadingSequentialCollection()); + } + } + + public void testSupportsOffloadingSequentialCollection() { + AB builder = createTestAggregatorBuilder(); + boolean supportsOffloadingSequentialCollection = builder.supportsOffloadingSequentialCollection(); + AggregationBuilder bucketBuilder = new HistogramAggregationBuilder("test"); + assertTrue(bucketBuilder.supportsOffloadingSequentialCollection()); + bucketBuilder.subAggregation(builder); + assertThat(bucketBuilder.supportsOffloadingSequentialCollection(), equalTo(supportsOffloadingSequentialCollection)); } /**