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

Support cancellation for admin apis #13966

Merged
merged 29 commits into from
Sep 3, 2024

Conversation

aasom143
Copy link
Contributor

@aasom143 aasom143 commented Jun 4, 2024

Description

Today, some APIs support only top-level timeouts, where a request is cancelled if it doesn't receive a response within the default 30-second timeout. However, the problem arises when these APIs internally make transport calls to cluster manager or other data nodes within the cluster even after the timeout expires. These internal/child calls, which are spawned by the API, are never timed out or cancelled, even though the top-level request was timed out. As a result, the cluster continues to perform unnecessary (wasteful) work and consuming resources but the user is not waiting for a response.

Related Issues

Resolves #13908

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • API changes companion pull request created.
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

Copy link
Contributor

github-actions bot commented Jun 4, 2024

❌ Gradle check result for da592e1: 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
Collaborator

@gaobinlong gaobinlong left a comment

Choose a reason for hiding this comment

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

Thanks for your work, but is there an issue related to your PR? It's better that each PR has a related PR to make sure everybody learn the idea and get more details about it, and please add some description about the code change in your PR to accelerate the reviewing process. In addition, please add a change log for this PR, and unit tests or yml tests for the code change are also needed.

Copy link
Contributor

❌ Gradle check result for 9152bbf: 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

github-actions bot commented Jul 4, 2024

❌ Gradle check result for 75cbb40: 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?

@github-actions github-actions bot added Cluster Manager enhancement Enhancement or improvement to existing feature or request labels Jul 4, 2024
…uest

Signed-off-by: Somesh Gupta <someshgupta987@gmail.com>
Copy link
Contributor

❌ Gradle check result for 6492189: 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

@rajiv-kv rajiv-kv left a comment

Choose a reason for hiding this comment

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

LGTM !

@rajiv-kv
Copy link
Contributor

❌ Gradle check result for 6492189: 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?

Please take a look at the failure

Signed-off-by: Somesh Gupta <35426854+aasom143@users.noreply.github.com>
Copy link
Contributor

❌ Gradle check result for f689a76: 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 6381e77: 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?

Signed-off-by: Somesh Gupta <someshgupta987@gmail.com>

Added license
Copy link
Contributor

❌ Gradle check result for 24d16a2: 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?

@shwetathareja shwetathareja added the backport 2.x Backport to 2.x branch label Sep 2, 2024
Copy link
Contributor

github-actions bot commented Sep 2, 2024

❌ Gradle check result for 24d16a2: 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

github-actions bot commented Sep 3, 2024

❕ Gradle check result for e062a97: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.index.shard.RemoteIndexShardTests.testIgnoreShardIdle

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@shwetathareja shwetathareja merged commit 9f81479 into opensearch-project:main Sep 3, 2024
32 of 34 checks passed
@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-13966-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 9f814797d0fe57457359a1f8a1b222cea80dc003
# Push it to GitHub
git push --set-upstream origin backport/backport-13966-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-13966-to-2.x.

aasom143 added a commit to aasom143/OpenSearch that referenced this pull request Sep 3, 2024
* Support cancellation for admin apis - add implementation for _cat/shards

Signed-off-by: Somesh Gupta <someshgupta987@gmail.com>
(cherry picked from commit 9f81479)
shwetathareja pushed a commit that referenced this pull request Sep 3, 2024
* Support cancellation for admin apis (#13966)

* Support cancellation for admin apis - add implementation for _cat/shards

Signed-off-by: Somesh Gupta <someshgupta987@gmail.com>
(cherry picked from commit 9f81479)
akolarkunnu pushed a commit to akolarkunnu/OpenSearch that referenced this pull request Sep 10, 2024
* Support cancellation for admin apis - add implementation for _cat/shards

Signed-off-by: Somesh Gupta <someshgupta987@gmail.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 Cluster Manager enhancement Enhancement or improvement to existing feature or request v2.17.0
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Feature Request] Support cancellation on runaway apis.
7 participants