Add dynamic index and cluster setting for concurrent segment search #7956

jed326 · 2023-06-07T17:24:41Z

Description

Add dynamic cluster and index setting to control concurrent segment search. Index level setting overrides cluster level setting.

Related Issues

Resolves #7356

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2023-06-07T17:41:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/17050/
CommitID: 0928733
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-06-07T17:57:16Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/17048/
CommitID: 2a6ab8b

codecov · 2023-06-07T18:02:44Z

Codecov Report

Merging #7956 (e61ba99) into main (28d5948) will decrease coverage by 0.56%.
The diff coverage is 91.42%.

@@             Coverage Diff              @@
##               main    #7956      +/-   ##
============================================
- Coverage     71.44%   70.88%   -0.56%     
+ Complexity    56913    56551     -362     
============================================
  Files          4715     4716       +1     
  Lines        267246   267268      +22     
  Branches      39186    39187       +1     
============================================
- Hits         190930   189466    -1464     
- Misses        60492    61824    +1332     
- Partials      15824    15978     +154

Impacted Files	Coverage Δ
.../org/opensearch/search/internal/SearchContext.java	`36.36% <0.00%> (-0.85%)`	⬇️
...va/org/opensearch/search/DefaultSearchContext.java	`78.57% <75.00%> (-0.12%)`	⬇️
...rg/opensearch/common/settings/ClusterSettings.java	`92.85% <100.00%> (+0.35%)`	⬆️
...pensearch/common/settings/IndexScopedSettings.java	`100.00% <100.00%> (ø)`
.../main/java/org/opensearch/index/IndexSettings.java	`86.66% <100.00%> (+0.06%)`	⬆️
.../main/java/org/opensearch/search/SearchModule.java	`97.76% <100.00%> (-0.02%)`	⬇️
...main/java/org/opensearch/search/SearchService.java	`71.90% <100.00%> (-0.74%)`	⬇️
...rch/search/query/ConcurrentQueryPhaseSearcher.java	`75.00% <100.00%> (+5.30%)`	⬆️
...search/search/query/QueryPhaseSearcherWrapper.java	`100.00% <100.00%> (ø)`

... and 462 files with indirect coverage changes

github-actions · 2023-06-07T18:17:47Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testIndexDeletionDuringSnapshotCreationInQueue
      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchTaskCancellationWithHighCpu
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

URL: https://build.ci.opensearch.org/job/gradle-check/17052/
CommitID: a412d57
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

jed326 · 2023-06-07T19:11:18Z

Tagging @sohami @reta @andrross for some help reviewing. Thanks!

sohami

LGTM. Thanks @jed326 for working on this

github-actions · 2023-06-09T23:16:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchTaskCancellationWithHighCpu
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testCancelPrimaryAllocation

URL: https://build.ci.opensearch.org/job/gradle-check/17263/
CommitID: d8a19dc
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

jed326 · 2023-06-10T01:50:07Z

Do we need a corresponding doc update for this issue? Seems important to note somewhere that the index setting overrides the cluster setting but not sure where that would go. Maybe could just track this as a part of opensearch-project/documentation-webstie#2662?

reta · 2023-06-10T12:16:28Z

Do we need a corresponding doc update for this issue?

Definitely, since we are adding new settings, those have to be documented, thanks @jed326

reta · 2023-06-12T13:19:35Z

@jed326 LGTM, one comment to cleanup the ConcurrentQueryPhaseSearcher but all good otherwise

Signed-off-by: Jay Deng <[email protected]>

server/src/main/java/org/opensearch/search/query/ConcurrentQueryPhaseSearcher.java

github-actions · 2023-06-12T17:15:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/17328/
CommitID: e61ba99

jed326 · 2023-06-12T17:31:21Z

@reta looks like tests are passing, I think we should be good to merge now. Thanks!

opensearch-trigger-bot · 2023-06-12T20:02:17Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-7956-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 068404ed1cef5e1d023816d57054aef98a7aa343
# Push it to GitHub
git push --set-upstream origin backport/backport-7956-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7956-to-2.x.

reta · 2023-06-12T20:04:25Z

@jed326 ah ... the auto 2.x backport failed, could you please submit the manual one? thank you

jed326 · 2023-06-12T20:05:22Z

@reta Sure will do

…pensearch-project#7956) * Add dynamic index and cluster setting for concurrent segment search Signed-off-by: Jay Deng <[email protected]> * Use feature flagged settings map Signed-off-by: Jay Deng <[email protected]> --------- Signed-off-by: Jay Deng <[email protected]>

jed326 · 2023-06-13T00:11:22Z

server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java

+        // Test that we throw an exception without the feature flag
+        SettingsModule module = new SettingsModule(Settings.EMPTY);
+        IndexScopedSettings indexScopedSettings = module.getIndexScopedSettings();
+        expectThrows(


https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java#L606

https://github.com/opensearch-project/OpenSearch/blob/2.x/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java#L608

This is an IllegalArgumentException in 2.x

…pensearch-project#7956) * Add dynamic index and cluster setting for concurrent segment search Signed-off-by: Jay Deng <[email protected]> * Use feature flagged settings map Signed-off-by: Jay Deng <[email protected]> --------- Signed-off-by: Jay Deng <[email protected]>

…7956) (#8034) * Add dynamic index and cluster setting for concurrent segment search * Use feature flagged settings map --------- Signed-off-by: Jay Deng <[email protected]>

…pensearch-project#7956) (opensearch-project#8034) * Add dynamic index and cluster setting for concurrent segment search * Use feature flagged settings map --------- Signed-off-by: Jay Deng <[email protected]>

…pensearch-project#7956) * Add dynamic index and cluster setting for concurrent segment search Signed-off-by: Jay Deng <[email protected]> * Use feature flagged settings map Signed-off-by: Jay Deng <[email protected]> --------- Signed-off-by: Jay Deng <[email protected]> Signed-off-by: Rishab Nahata <[email protected]>

martin-gaievski · 2023-08-30T17:05:34Z

@jed326 it seems that feature flag has not been removed for concurrent_search, does this mean that to enable it we need both, feature flag and new index/cluster setting?

jed326 · 2023-08-30T19:26:11Z

@martin-gaievski Yes the feature flag is not removed. The cluster setting defaults to true so concurrent search should be enabled by default once the feature flag is enabled.

…pensearch-project#7956) * Add dynamic index and cluster setting for concurrent segment search Signed-off-by: Jay Deng <[email protected]> * Use feature flagged settings map Signed-off-by: Jay Deng <[email protected]> --------- Signed-off-by: Jay Deng <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

jed326 changed the base branch from main to 2.x June 7, 2023 17:39

jed326 changed the base branch from 2.x to main June 7, 2023 17:39

jed326 force-pushed the cs-dynamic branch from 0928733 to a412d57 Compare June 7, 2023 18:08

jed326 marked this pull request as ready for review June 7, 2023 19:10

jed326 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and dbwiddis as code owners June 7, 2023 19:10

sohami approved these changes Jun 9, 2023

View reviewed changes

Use feature flagged settings map

e61ba99

Signed-off-by: Jay Deng <[email protected]>

jed326 force-pushed the cs-dynamic branch from d8a19dc to e61ba99 Compare June 12, 2023 16:42

reta reviewed Jun 12, 2023

View reviewed changes

server/src/main/java/org/opensearch/search/query/ConcurrentQueryPhaseSearcher.java Show resolved Hide resolved

reta approved these changes Jun 12, 2023

View reviewed changes

jed326 mentioned this pull request Jun 12, 2023

[DOC] Concurrent Segment Search support in OpenSearch opensearch-project/documentation-website#3662

Closed

4 tasks

reta merged commit 068404e into opensearch-project:main Jun 12, 2023

reta added the backport 2.x Backport to 2.x branch label Jun 12, 2023

jed326 mentioned this pull request Jun 12, 2023

[Backport 2.x] Add dynamic index and cluster setting for concurrent segment search #8034

Merged

6 tasks

jed326 commented Jun 13, 2023

View reviewed changes

jed326 deleted the cs-dynamic branch September 13, 2023 19:09

martin-gaievski mentioned this pull request Sep 28, 2023

Consumed latest changes from core, use QueryPhaseSearcherWrapper as parent class for Hybrid QPS opensearch-project/neural-search#356

Merged

2 tasks

sohami mentioned this pull request May 21, 2024

Add Jay Deng as a maintainer #13773

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dynamic index and cluster setting for concurrent segment search #7956

Add dynamic index and cluster setting for concurrent segment search #7956

jed326 commented Jun 7, 2023

github-actions bot commented Jun 7, 2023

github-actions bot commented Jun 7, 2023

codecov bot commented Jun 7, 2023 •

edited

Loading

github-actions bot commented Jun 7, 2023

jed326 commented Jun 7, 2023 •

edited

Loading

sohami left a comment

github-actions bot commented Jun 9, 2023

jed326 commented Jun 10, 2023

reta commented Jun 10, 2023

reta commented Jun 12, 2023

github-actions bot commented Jun 12, 2023

jed326 commented Jun 12, 2023

opensearch-trigger-bot bot commented Jun 12, 2023

reta commented Jun 12, 2023

jed326 commented Jun 12, 2023

jed326 Jun 13, 2023

martin-gaievski commented Aug 30, 2023

jed326 commented Aug 30, 2023

Add dynamic index and cluster setting for concurrent segment search #7956

Add dynamic index and cluster setting for concurrent segment search #7956

Conversation

jed326 commented Jun 7, 2023

Description

Related Issues

Check List

github-actions bot commented Jun 7, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 7, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Jun 7, 2023 • edited Loading

Codecov Report

github-actions bot commented Jun 7, 2023

Gradle Check (Jenkins) Run Completed with:

jed326 commented Jun 7, 2023 • edited Loading

sohami left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 9, 2023

Gradle Check (Jenkins) Run Completed with:

jed326 commented Jun 10, 2023

reta commented Jun 10, 2023

reta commented Jun 12, 2023

github-actions bot commented Jun 12, 2023

Gradle Check (Jenkins) Run Completed with:

jed326 commented Jun 12, 2023

opensearch-trigger-bot bot commented Jun 12, 2023

reta commented Jun 12, 2023

jed326 commented Jun 12, 2023

jed326 Jun 13, 2023

Choose a reason for hiding this comment

martin-gaievski commented Aug 30, 2023

jed326 commented Aug 30, 2023

codecov bot commented Jun 7, 2023 •

edited

Loading

jed326 commented Jun 7, 2023 •

edited

Loading