diff --git a/CHANGELOG.md b/CHANGELOG.md index a34eeb5be7d5c..101a3718d6161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - Change the default max header size from 8KB to 16KB. ([#18024](https://github.com/opensearch-project/OpenSearch/pull/18024)) +- Enable concurrent_segment_search auto mode by default[#17978](https://github.com/opensearch-project/OpenSearch/pull/17978) ### Dependencies diff --git a/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/30_single_value_field.yml b/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/30_single_value_field.yml index 77e8bf6359f22..c18b1e5fc0448 100644 --- a/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/30_single_value_field.yml +++ b/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/30_single_value_field.yml @@ -145,7 +145,8 @@ setup: - match: {hits.total: 15} - match: {aggregations.mfs.doc_count: 14} - match: {aggregations.mfs.fields.0.count: 14} - - match: {aggregations.mfs.fields.2.correlation.val2: 0.9569513137793205} + - gte: {aggregations.mfs.fields.2.correlation.val2: 0.956951313779319} + - lte: {aggregations.mfs.fields.2.correlation.val2: 0.956951313779321} --- "Partially unmapped with missing default": @@ -159,7 +160,8 @@ setup: - match: {hits.total: 15} - match: {aggregations.mfs.doc_count: 15} - match: {aggregations.mfs.fields.0.count: 15} - - match: {aggregations.mfs.fields.2.correlation.val2: 0.9567970467908384} + - gte: {aggregations.mfs.fields.2.correlation.val2: 0.956797046790837} + - lte: {aggregations.mfs.fields.2.correlation.val2: 0.956797046790839} --- "With script": diff --git a/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/40_multi_value_field.yml b/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/40_multi_value_field.yml index 467efce78a467..218976c3fbbdf 100644 --- a/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/40_multi_value_field.yml +++ b/modules/aggs-matrix-stats/src/yamlRestTest/resources/rest-api-spec/test/stats/40_multi_value_field.yml @@ -132,7 +132,8 @@ setup: - match: {hits.total: 15} - match: {aggregations.mfs.doc_count: 14} - match: {aggregations.mfs.fields.0.count: 14} - - match: {aggregations.mfs.fields.0.correlation.val1: 0.06838646533369998} + - gte: {aggregations.mfs.fields.0.correlation.val1: 0.068386465333698} + - lte: {aggregations.mfs.fields.0.correlation.val1: 0.068386465333701} --- "Multi value field Min": @@ -146,7 +147,8 @@ setup: - match: {hits.total: 15} - match: {aggregations.mfs.doc_count: 14} - match: {aggregations.mfs.fields.0.count: 14} - - match: {aggregations.mfs.fields.0.correlation.val1: -0.09777682707831963} + - gte: {aggregations.mfs.fields.0.correlation.val1: -0.097776827078320} + - lte: {aggregations.mfs.fields.0.correlation.val1: -0.097776827078318} --- "Partially unmapped": @@ -160,7 +162,8 @@ setup: - match: {hits.total: 15} - match: {aggregations.mfs.doc_count: 13} - match: {aggregations.mfs.fields.0.count: 13} - - match: {aggregations.mfs.fields.0.correlation.val1: -0.044997535185684244} + - gte: {aggregations.mfs.fields.0.correlation.val1: -0.044997535185685} + - lte: {aggregations.mfs.fields.0.correlation.val1: -0.044997535185683} --- "Partially unmapped with missing defaults": @@ -174,7 +177,8 @@ setup: - match: {hits.total: 15} - match: {aggregations.mfs.doc_count: 15} - match: {aggregations.mfs.fields.0.count: 15} - - match: {aggregations.mfs.fields.0.correlation.val2: 0.04028024709708195} + - gte: {aggregations.mfs.fields.0.correlation.val2: 0.040280247097080} + - lte: {aggregations.mfs.fields.0.correlation.val2: 0.040280247097082} --- "With script": diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/memory/breaker/CircuitBreakerServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/memory/breaker/CircuitBreakerServiceIT.java index 441259c3ba41a..a255ce40f9c1b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/memory/breaker/CircuitBreakerServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/memory/breaker/CircuitBreakerServiceIT.java @@ -110,7 +110,7 @@ public static Collection parameters() { protected Settings nodeSettings(int nodeOrdinal) { return Settings.builder() .put(super.nodeSettings(nodeOrdinal)) - .put(SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY, randomIntBetween(1, 2)) + .put(SearchService.CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT_KEY, randomIntBetween(1, 2)) .build(); } diff --git a/server/src/internalClusterTest/java/org/opensearch/search/stats/ConcurrentSearchStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/search/stats/ConcurrentSearchStatsIT.java index f8d2955440bc4..eb93abf7c38f2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/stats/ConcurrentSearchStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/stats/ConcurrentSearchStatsIT.java @@ -59,7 +59,7 @@ protected Settings nodeSettings(int nodeOrdinal) { .put(super.nodeSettings(nodeOrdinal)) .put(IndicesService.INDICES_CACHE_CLEAN_INTERVAL_SETTING.getKey(), "1ms") .put(IndicesQueryCache.INDICES_QUERIES_CACHE_ALL_SEGMENTS_SETTING.getKey(), true) - .put(SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY, SEGMENT_SLICE_COUNT) + .put(SearchService.CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT_KEY, SEGMENT_SLICE_COUNT) .put(SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true) .build(); } diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 578304c77fef9..26a4d9d8469a3 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -78,10 +78,11 @@ import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING; import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION; +import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_DEFAULT_SLICE_COUNT_VALUE; +import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MIN_SLICE_COUNT_VALUE; import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MODE_ALL; import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MODE_AUTO; import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MODE_NONE; -import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE; /** * This class encapsulates all index level settings and handles settings updates. @@ -726,8 +727,8 @@ public static IndexMergePolicy fromString(String text) { public static final Setting INDEX_CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT = Setting.intSetting( "index.search.concurrent.max_slice_count", - CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE, - CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE, + CONCURRENT_SEGMENT_SEARCH_DEFAULT_SLICE_COUNT_VALUE, + CONCURRENT_SEGMENT_SEARCH_MIN_SLICE_COUNT_VALUE, Property.Dynamic, Property.IndexScope ); diff --git a/server/src/main/java/org/opensearch/search/DefaultSearchContext.java b/server/src/main/java/org/opensearch/search/DefaultSearchContext.java index 854a8dee56e6d..a913f943cd19c 100644 --- a/server/src/main/java/org/opensearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/opensearch/search/DefaultSearchContext.java @@ -50,6 +50,8 @@ import org.opensearch.common.SetOnce; import org.opensearch.common.lease.Releasables; import org.opensearch.common.lucene.search.Queries; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.BigArrays; import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; @@ -1056,46 +1058,77 @@ public BucketCollectorProcessor bucketCollectorProcessor() { } /** - * Evaluate the concurrentSearchMode based on cluster and index settings if concurrent segment search - * should be used for this request context - * If the cluster.search.concurrent_segment_search.mode setting - * is not explicitly set, the evaluation falls back to the - * cluster.search.concurrent_segment_search.enabled boolean setting - * which will evaluate to true or false. This is then evaluated to "all" or "none" respectively - * @return one of "none", "auto", "all" + * Determines the appropriate concurrent segment search mode for the current search request. + *

+ * This method evaluates both index-level and cluster-level settings to decide whether + * concurrent segment search should be enabled. The resolution logic is as follows: + *

    + *
  1. If the request targets a system index or uses search throttling, concurrent segment search is disabled.
    2. + *
  2. If a legacy boolean setting is present (cluster or index level), it is honored: + *
      + *
    • true → enables concurrent segment search ("all")
      • + *
    • false → disables it ("none")
      • + *
    + *
    3. + *
  3. Otherwise, the modern string-based setting is used. Allowed values are: "none", "auto", or "all".
    4. + *
+ * + * @param concurrentSearchExecutor the executor used for concurrent segment search; if null, disables the feature + * @return the resolved concurrent segment search mode: "none", "auto", or "all" */ private String evaluateConcurrentSearchMode(Executor concurrentSearchExecutor) { - // Do not use concurrent segment search for system indices or throttled requests. See: - // https://github.com/opensearch-project/OpenSearch/issues/12951 - if (indexShard.isSystem() || indexShard.indexSettings().isSearchThrottled()) { + // Skip concurrent search for system indices, throttled requests, or if dependencies are missing + if (indexShard.isSystem() + || indexShard.indexSettings().isSearchThrottled() + || clusterService == null + || concurrentSearchExecutor == null) { return CONCURRENT_SEGMENT_SEARCH_MODE_NONE; } - if ((clusterService != null) && concurrentSearchExecutor != null) { - String concurrentSearchMode = indexService.getIndexSettings() - .getSettings() - .get( - IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_MODE.getKey(), - clusterService.getClusterSettings().getOrNull(CLUSTER_CONCURRENT_SEGMENT_SEARCH_MODE) - ); - if (concurrentSearchMode != null) { - return concurrentSearchMode; - } - // mode setting not set, fallback to concurrent_segment_search.enabled setting - return indexService.getIndexSettings() - .getSettings() - .getAsBoolean( - IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), - clusterService.getClusterSettings().get(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING) - ) ? CONCURRENT_SEGMENT_SEARCH_MODE_ALL : CONCURRENT_SEGMENT_SEARCH_MODE_NONE; + Settings indexSettings = indexService.getIndexSettings().getSettings(); + ClusterSettings clusterSettings = clusterService.getClusterSettings(); + final String newConcurrentMode = indexSettings.get( + IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_MODE.getKey(), + clusterSettings.getOrNull(CLUSTER_CONCURRENT_SEGMENT_SEARCH_MODE) + ); + + // Step 1: New concurrent mode is explicitly set → use it + if (newConcurrentMode != null) return newConcurrentMode; + + final Boolean legacySetting = indexSettings.getAsBoolean( + IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), + clusterSettings.getOrNull(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING) + ); + + // Step 2: If new concurrent setting is not explicitly set + // then check if Legacy boolean setting is explicitly set → use it + if (legacySetting != null) { + return Boolean.TRUE.equals(legacySetting) ? CONCURRENT_SEGMENT_SEARCH_MODE_ALL : CONCURRENT_SEGMENT_SEARCH_MODE_NONE; } - return CONCURRENT_SEGMENT_SEARCH_MODE_NONE; + + // Step 3: Neither explicitly set → use default concurrent mode + return indexSettings.get( + IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_MODE.getKey(), + clusterSettings.get(CLUSTER_CONCURRENT_SEGMENT_SEARCH_MODE) + ); } + /** + * Returns the target maximum slice count to use for concurrent segment search. + * + * If concurrent segment search is disabled (either due to system index, throttled search, + * missing executor, or explicitly disabled settings), then we return a slice count of 1. + * This effectively disables concurrent slicing and ensures that the search is performed + * in a single-threaded manner. + * + * Otherwise, fetch the configured slice count from index or cluster-level settings. + * + * @return number of slices to use for concurrent segment search; returns 1 if concurrent search is disabled. + */ @Override public int getTargetMaxSliceCount() { if (shouldUseConcurrentSearch() == false) { - throw new IllegalStateException("Target slice count should not be used when concurrent search is disabled"); + return 1; // Disable slicing: run search in a single thread when concurrent search is off } return indexService.getIndexSettings() diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index 1fa0fca68fca1..d3b00ae8fb0c8 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -279,7 +279,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv public static final Setting CLUSTER_CONCURRENT_SEGMENT_SEARCH_MODE = Setting.simpleString( "search.concurrent_segment_search.mode", - CONCURRENT_SEGMENT_SEARCH_MODE_NONE, + CONCURRENT_SEGMENT_SEARCH_MODE_AUTO, value -> { switch (value) { case CONCURRENT_SEGMENT_SEARCH_MODE_ALL: @@ -297,18 +297,18 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv // settings to configure maximum slice created per search request using OS custom slice computation mechanism. Default lucene // mechanism will not be used if this setting is set with value > 0 - public static final String CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY = "search.concurrent.max_slice_count"; - public static final int CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE = 0; + public static final String CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT_KEY = "search.concurrent.max_slice_count"; + public static final int CONCURRENT_SEGMENT_SEARCH_DEFAULT_SLICE_COUNT_VALUE = computeDefaultSliceCount(); + public static final int CONCURRENT_SEGMENT_SEARCH_MIN_SLICE_COUNT_VALUE = 0; // value == 0 means lucene slice computation will be used public static final Setting CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_SETTING = Setting.intSetting( - CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY, - CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE, - CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE, + CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT_KEY, + CONCURRENT_SEGMENT_SEARCH_DEFAULT_SLICE_COUNT_VALUE, + CONCURRENT_SEGMENT_SEARCH_MIN_SLICE_COUNT_VALUE, Property.Dynamic, Property.NodeScope ); - // value 0 means rewrite filters optimization in aggregations will be disabled @ExperimentalApi public static final Setting MAX_AGGREGATION_REWRITE_FILTERS = Setting.intSetting( @@ -1871,6 +1871,27 @@ public MinAndMax estimatedMinAndMax() { } } + /** + * Computes the default maximum number of slices for concurrent segment search. + *

+ * This value is dynamically calculated as: + * 

+     *     min(availableProcessors / 2, 4)
+     *
+ * This ensures that: + *
    + *
  • On small machines, it avoids over-threading.
    • + *
  • On larger machines, it caps the concurrency to a reasonable level (4 slices).
    • + *
+ * This default is used when the user does not explicitly set the + * {@code search.concurrent.max_slice_count} cluster setting. + * + * @return the computed default slice count + */ + private static int computeDefaultSliceCount() { + return Math.max(1, Math.min(Runtime.getRuntime().availableProcessors() / 2, 4)); + } + /** * This helper class ensures we only execute either the success or the failure path for {@link SearchOperationListener}. * This is crucial for some implementations like {@link org.opensearch.index.search.stats.ShardSearchStats}. diff --git a/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java b/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java index 4e02ee13a50e3..e645cd6a7723f 100644 --- a/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java +++ b/server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java @@ -341,7 +341,7 @@ public void testSlicesInternal() throws Exception { // Case 1: Verify the slice count when lucene default slice computation is used IndexSearcher.LeafSlice[] slices = searcher.slicesInternal( leaves, - SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_DEFAULT_VALUE + SearchService.CONCURRENT_SEGMENT_SEARCH_MIN_SLICE_COUNT_VALUE ); int expectedSliceCount = 2; // 2 slices will be created since max segment per slice of 5 will be reached diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index 6e47f7aa29345..3a5b2e84d81a5 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -1958,7 +1958,7 @@ protected Settings nodeSettings(int nodeOrdinal) { .putList(DISCOVERY_SEED_PROVIDERS_SETTING.getKey(), "file") // By default, for tests we will put the target slice count of 2. This will increase the probability of having multiple slices // when tests are run with concurrent segment search enabled - .put(SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY, 2) + .put(SearchService.CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT_KEY, 2) .put(featureFlagSettings()); // Enable tracer only when Telemetry Setting is enabled diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java index a3d5166b23fb8..0879eacb938c1 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java @@ -257,7 +257,7 @@ private Node newNode() { .put(TelemetrySettings.TRACER_FEATURE_ENABLED_SETTING.getKey(), true) // By default, for tests we will put the target slice count of 2. This will increase the probability of having multiple slices // when tests are run with concurrent segment search enabled - .put(SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY, 2) + .put(SearchService.CONCURRENT_SEGMENT_SEARCH_MAX_SLICE_COUNT_KEY, 2) .put(nodeSettings()) // allow test cases to provide their own settings or override these .put(featureFlagSettings);