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.

Sign up for GitHub

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

Jump to bottom

Avoid invalid retries on multiple replicas when querying #17370

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Avoid invalid retries on multiple replicas when querying #17370

wants to merge 1 commit into from
+107 −13

Conversation

kkewwei
Copy link
Contributor

@kkewwei kkewwei commented Feb 16, 2025 •
edited
Loading

Description

Related Issues

Resolves #17361

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.

@github-actions github-actions bot added Cluster Manager enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc labels Feb 16, 2025
@kkewwei kkewwei force-pushed the avoid_invalid_query branch from 19b332b to f235073 Compare February 16, 2025 08:39
@github-actions GitHub Actions
Copy link
Contributor

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

@kkewwei
Copy link
Contributor Author

kkewwei commented Feb 16, 2025

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

MetadataCreateIndexServiceTests.testCreateIndexWithContextDisabled #17291

@kkewwei kkewwei mentioned this pull request Feb 16, 2025
Closed
3 tasks
@opensearch-ci-bot opensearch-ci-bot mentioned this pull request Feb 16, 2025
Closed
@kkewwei
Copy link
Contributor Author

kkewwei commented Feb 18, 2025

@msfroh, please have a look in your spare time. Thank you very much.

msfroh
msfroh reviewed Feb 18, 2025
View reviewed changes
Copy link
Collaborator

@msfroh msfroh left a comment

Choose a reason for hiding this comment

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

Thanks a lot @kkewwei ! This is a nice improvement.

I left a couple of minor cleanup comments. Overall, it looks good.

@kkewwei kkewwei force-pushed the avoid_invalid_query branch 3 times, most recently from 589f4d4 to c34cedc Compare February 19, 2025 01:21
@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for c34cedc: null

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?

This was referenced Feb 19, 2025
Open
Open
@kkewwei
Copy link
Contributor Author

kkewwei commented Feb 19, 2025 •
edited
Loading

❌ Gradle check result for c34cedc: null

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?

The flaky test may be related to #17357, I'm still finding the reason.

@kkewwei
Copy link
Contributor Author

kkewwei commented Feb 20, 2025

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

The flaky test may be related to #17357, I'm still finding the reason.

PendingTasksBlocksIT.testPendingTasksWithClusterNotRecoveredBlock #17369

@github-actions GitHub Actions
Copy link
Contributor

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

@kkewwei kkewwei force-pushed the avoid_invalid_query branch from 17a63ef to ea2e674 Compare February 20, 2025 14:23
kkewwei
kkewwei commented Feb 20, 2025
View reviewed changes
server/src/main/java/org/opensearch/action/support/TransportActions.java
return (OpenSearchException.status(e).getStatus() / 100 != 4) && (e.getCause() instanceof TaskCancelledException == false)
// There exists a scenario where a primary shard (0 replicas) relocates and is in POST_RECOVERY on the
// target node but already deleted on the source node. Search request should still work.
|| (e.getCause() instanceof IndexNotFoundException);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@msfroh please help check whether is ok or not:
IndexNotFoundException is supported for testSearchWithRelocationAndSlowClusterStateProcessing
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would 429 circuit breaker exceptions be considered non-retriable?

@kkewwei
Copy link
Contributor Author

kkewwei commented Feb 20, 2025

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

RemotePrimaryRelocationIT.testMixedModeRelocation_FailInFinalize #17364
ResourceAwareTasksTests.testTaskResourceTrackingDuringTaskCancellation #14293

@kkewwei kkewwei force-pushed the avoid_invalid_query branch from ea2e674 to fcfb2ba Compare February 20, 2025 14:42
@github-actions GitHub Actions
Copy link
Contributor

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

@kkewwei kkewwei force-pushed the avoid_invalid_query branch from fcfb2ba to e965796 Compare February 20, 2025 16:02
@github-actions GitHub Actions
Copy link
Contributor

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

This was referenced Feb 20, 2025
Open
Open
@kkewwei
Avoid invalid retries on multiple replicas when querying
d278191 
Signed-off-by: kkewwei <kewei.11@bytedance.com>
Signed-off-by: kkewwei <kkewwei@163.com>
@kkewwei kkewwei force-pushed the avoid_invalid_query branch from e965796 to d278191 Compare February 21, 2025 07:58
@github-actions GitHub Actions
Copy link
Contributor

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

@opensearch-ci-bot opensearch-ci-bot mentioned this pull request Feb 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msfroh msfroh msfroh left review comments

@anasalkouz anasalkouz Awaiting requested review from anasalkouz anasalkouz is a code owner

@andrross andrross Awaiting requested review from andrross andrross is a code owner

@ashking94 ashking94 Awaiting requested review from ashking94 ashking94 is a code owner

@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner

@CEHENKLE CEHENKLE Awaiting requested review from CEHENKLE CEHENKLE is a code owner

@cwperks cwperks Awaiting requested review from cwperks cwperks is a code owner

@dblock dblock Awaiting requested review from dblock dblock is a code owner

@dbwiddis dbwiddis Awaiting requested review from dbwiddis dbwiddis is a code owner

@gbbafna gbbafna Awaiting requested review from gbbafna gbbafna is a code owner

@jainankitk jainankitk Awaiting requested review from jainankitk jainankitk is a code owner

@kotwanikunal kotwanikunal Awaiting requested review from kotwanikunal kotwanikunal is a code owner

@linuxpi linuxpi Awaiting requested review from linuxpi linuxpi is a code owner

@mch2 mch2 Awaiting requested review from mch2 mch2 is a code owner

@nknize nknize Awaiting requested review from nknize nknize is a code owner

@owaiskazi19 owaiskazi19 Awaiting requested review from owaiskazi19 owaiskazi19 is a code owner

@reta reta Awaiting requested review from reta reta is a code owner

@Rishikesh1159 Rishikesh1159 Awaiting requested review from Rishikesh1159 Rishikesh1159 is a code owner

@sachinpkale sachinpkale Awaiting requested review from sachinpkale sachinpkale is a code owner

@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

@shwetathareja shwetathareja Awaiting requested review from shwetathareja shwetathareja is a code owner

@sohami sohami Awaiting requested review from sohami sohami is a code owner

@VachaShah VachaShah Awaiting requested review from VachaShah VachaShah is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Cluster Manager enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc
Projects
Cluster Manager Project Board
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Avoid invalid retries on multiple replicas when querying
3 participants
@kkewwei @msfroh @Bukhtawar