Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation for the search backpressure cancellation settings #15501

Merged
merged 10 commits into from
Sep 17, 2024

Conversation

gaobinlong
Copy link
Collaborator

@gaobinlong gaobinlong commented Aug 29, 2024

Description

Updating the cluster level setting search_backpressure.cancellation_burst(deprecated), search_backpressure.search_task.cancellation_burst or search_backpressure.search_shard_task.cancellation_burst to an non-default value cause the cluster crash, the reason is that the value of cancellationRate is always 0 here:


, so an exception is thrown here:
.

Just like cancellationRatio, we also need to get the value of cancellationRate from the cluster settings and pass it to the constructor method of SearchBackpressureState.

Another issue is that we don't have some validations for the setting cancellationRate and cancellationRatio, so when updating these two settings to 0, the same exception "rate must be greater than zero" is also thrown and then crash the cluster.

Related Issues

#15495

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Signed-off-by: Gao Binlong <gbinlong@amazon.com>
@gaobinlong gaobinlong added the backport 2.x Backport to 2.x branch label Aug 29, 2024
@gaobinlong
Copy link
Collaborator Author

@reta @dblock could you help to review this PR, thank you!

Copy link
Contributor

❌ Gradle check result for adcf328: FAILURE

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?

Copy link
Contributor

❌ Gradle check result for f518b2e: FAILURE

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?

Copy link
Contributor

✅ Gradle check result for e016a42: SUCCESS

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Copy link
Collaborator

@reta reta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @gaobinlong ! @andrross any concerns you might have? thanks!

Copy link
Contributor

❌ Gradle check result for 03b13b3: FAILURE

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?

Copy link
Contributor

✅ Gradle check result for 03b13b3: SUCCESS

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Copy link
Contributor

✅ Gradle check result for 575303c: SUCCESS

@reta reta added the v2.18.0 Issues and PRs related to version 2.18.0 label Sep 17, 2024
@reta reta merged commit 8347d0e into opensearch-project:main Sep 17, 2024
38 of 40 checks passed
@reta
Copy link
Collaborator

reta commented Sep 17, 2024

Sorry @gaobinlong , completely forgot about this change

@opensearch-trigger-bot
Copy link
Contributor

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:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-15501-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8347d0ec22aa865dea3bc4b2027fb7bdf486fab2
# Push it to GitHub
git push --set-upstream origin backport/backport-15501-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

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

@reta
Copy link
Collaborator

reta commented Sep 17, 2024

@gaobinlong could you please backport to 2.x manually? thank you!

gaobinlong added a commit to gaobinlong/OpenSearch that referenced this pull request Sep 18, 2024
…nsearch-project#15501)

* Fix updating search backpressure settings crashing the cluster

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Modify change log

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix version check

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Increase test coverage

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Format the code

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Optimize some code

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix typo

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
(cherry picked from commit 8347d0e)
sachinpkale pushed a commit to sachinpkale/OpenSearch that referenced this pull request Sep 19, 2024
…nsearch-project#15501)

* Fix updating search backpressure settings crashing the cluster

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Modify change log

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix version check

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Increase test coverage

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Format the code

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Optimize some code

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix typo

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
reta pushed a commit that referenced this pull request Sep 19, 2024
…n settings (#15969)

* Add validation for the search backpressure cancellation settings (#15501)

* Fix updating search backpressure settings crashing the cluster

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Modify change log

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix version check

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Increase test coverage

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Format the code

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Optimize some code

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix typo

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
(cherry picked from commit 8347d0e)

* Update version check in yml test file

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport-failed v2.18.0 Issues and PRs related to version 2.18.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants