From 9d7274db8f6e0a3cecb3c8b970eed842925d22b9 Mon Sep 17 00:00:00 2001 From: Sorabh Date: Tue, 20 Feb 2024 14:40:47 -0800 Subject: [PATCH] Disable concurrent search path for composite aggregations. For more details see: https://github.com/opensearch-project/OpenSearch/issues/12331 (#12375) Signed-off-by: Sorabh Hamirwasia Signed-off-by: Aman Khare --- .../opensearch.release-notes-2.12.0.md | 1 + .../aggregations/bucket/CompositeAggIT.java | 99 +++++++++++++++++++ .../CompositeAggregationFactory.java | 3 +- 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/CompositeAggIT.java diff --git a/release-notes/opensearch.release-notes-2.12.0.md b/release-notes/opensearch.release-notes-2.12.0.md index b5bd7767b8294..b188efe2c6bfe 100644 --- a/release-notes/opensearch.release-notes-2.12.0.md +++ b/release-notes/opensearch.release-notes-2.12.0.md @@ -142,6 +142,7 @@ - [Query Insights] Implement Top N Queries feature to collect and gather information about high latency queries in a window ([#11904](https://github.com/opensearch-project/OpenSearch/pull/11904)) - Add override support for sampling based on action ([#9621](https://github.com/opensearch-project/OpenSearch/issues/9621)) - Added custom sampler support based on transport action in request ([#9621](https://github.com/opensearch-project/OpenSearch/issues/9621)) +- Disable concurrent search for composite aggregation([#12375](https://github.com/opensearch-project/OpenSearch/pull/12375)) ### Removed - Remove deprecated classes for Rounding ([#10956](https://github.com/opensearch-project/OpenSearch/issues/10956)) diff --git a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/CompositeAggIT.java b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/CompositeAggIT.java new file mode 100644 index 0000000000000..5a38ba670f1dc --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/CompositeAggIT.java @@ -0,0 +1,99 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.search.aggregations.bucket; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; + +import org.opensearch.action.search.SearchResponse; +import org.opensearch.cluster.health.ClusterHealthStatus; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.settings.Settings; +import org.opensearch.search.aggregations.bucket.composite.CompositeAggregationBuilder; +import org.opensearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder; +import org.opensearch.search.aggregations.bucket.composite.TermsValuesSourceBuilder; +import org.opensearch.search.aggregations.metrics.MaxAggregationBuilder; +import org.opensearch.test.OpenSearchIntegTestCase; +import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse; + +@OpenSearchIntegTestCase.SuiteScopeTestCase +public class CompositeAggIT extends ParameterizedStaticSettingsOpenSearchIntegTestCase { + + public CompositeAggIT(Settings staticSettings) { + super(staticSettings); + } + + @ParametersFactory + public static Collection parameters() { + return Arrays.asList( + new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() }, + new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() } + ); + } + + @Override + public void setupSuiteScopeCluster() throws Exception { + assertAcked( + prepareCreate( + "idx", + Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + ).setMapping("type", "type=keyword", "num", "type=integer", "score", "type=integer") + ); + waitForRelocation(ClusterHealthStatus.GREEN); + + client().prepareIndex("idx").setId("1").setSource("type", "type1", "num", "1", "score", "5").get(); + client().prepareIndex("idx").setId("1").setSource("type", "type2", "num", "11", "score", "50").get(); + refresh("idx"); + client().prepareIndex("idx").setId("1").setSource("type", "type1", "num", "1", "score", "2").get(); + client().prepareIndex("idx").setId("1").setSource("type", "type2", "num", "12", "score", "20").get(); + refresh("idx"); + client().prepareIndex("idx").setId("1").setSource("type", "type1", "num", "3", "score", "10").get(); + client().prepareIndex("idx").setId("1").setSource("type", "type2", "num", "13", "score", "15").get(); + refresh("idx"); + client().prepareIndex("idx").setId("1").setSource("type", "type1", "num", "3", "score", "1").get(); + client().prepareIndex("idx").setId("1").setSource("type", "type2", "num", "13", "score", "100").get(); + refresh("idx"); + + waitForRelocation(ClusterHealthStatus.GREEN); + refresh(); + } + + public void testCompositeAggWithNoSubAgg() { + SearchResponse rsp = client().prepareSearch("idx") + .addAggregation(new CompositeAggregationBuilder("my_composite", getTestValueSources())) + .get(); + assertSearchResponse(rsp); + } + + public void testCompositeAggWithSubAgg() { + SearchResponse rsp = client().prepareSearch("idx") + .addAggregation( + new CompositeAggregationBuilder("my_composite", getTestValueSources()).subAggregation( + new MaxAggregationBuilder("max").field("score") + ) + ) + .get(); + assertSearchResponse(rsp); + } + + private List> getTestValueSources() { + final List> sources = new ArrayList<>(); + sources.add(new TermsValuesSourceBuilder("keyword_vs").field("type")); + sources.add(new TermsValuesSourceBuilder("num_vs").field("num")); + return sources; + } +} diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregationFactory.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregationFactory.java index 2ff79fb623def..4af14ab014db5 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregationFactory.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregationFactory.java @@ -80,6 +80,7 @@ protected Aggregator createInternal( @Override protected boolean supportsConcurrentSegmentSearch() { - return true; + // See https://github.com/opensearch-project/OpenSearch/issues/12331 for details + return false; } }