Cancel in-flight search tasks due to search backpressure with 429 sta… #6634

PritLadani · 2023-03-11T10:14:15Z

Description

Added support for cancelling tasks with arbitrary status codes
Modified search backpressure service to cancel tasks with 429 status code

Issues Resolved

[List any issues this PR will resolve]

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.

…tus code Signed-off-by: PritLadani <pritkladani@gmail.com>

github-actions · 2023-03-11T10:36:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12289/
CommitID: 08eb7b7
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?

gbbafna · 2023-03-11T12:27:56Z

server/src/main/java/org/opensearch/OpenSearchException.java

+        if (cause.getCause() instanceof TaskCancelledException
+            && ((TaskCancelledException) cause.getCause()).status() == RestStatus.TOO_MANY_REQUESTS) {
+            return ((TaskCancelledException) cause.getCause()).status();


can we avoid TaskCancelledException in OpenSearchException ? This looks like an anti pattern.

One alternate here is to make it generic and throw the underlying cause's status in all cases till level 2 . We need to think about whether it is the right behavior and also make sure it doesn't cause any regression in existing code (ITs would make sure of that). Something on the below lines.

public RestStatus status() { Throwable cause = unwrapCause(); if (cause == this) { return RestStatus.INTERNAL_SERVER_ERROR; } else { if (cause.getCause() != cause) { return ExceptionsHelper.status(cause.getCause()); } return ExceptionsHelper.status(cause); } }

trying this

The other better option is to override this in TaskCancelledException .

@PritLadani Can you explore the second suggestion to do the override in TaskCancelledException? The code snippet above doesn't look quite right because it calls an "unwrapCause()" method but then proceeds to do another level of cause unwrapping. I'm really concerned about unintended side effects of that approach, and I also don't have a ton of confidence that ITs would catch every possible regression.

gbbafna · 2023-03-11T12:56:28Z

server/src/main/java/org/opensearch/tasks/TaskCancellationService.java

+    }
+
+    private static boolean isRejection(String reason) {
+        return (reason.contains("usage exceeded") || REASON_PARENT_CANCELLED_HIGH_RESOURCE_CONSUMPTION.equals(reason));


please make usage exceeded a constant and use that instead.

Signed-off-by: PritLadani <pritkladani@gmail.com>

github-actions · 2023-03-11T14:43:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12291/
CommitID: dfaae2d
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-03-11T14:50:21Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12292/
CommitID: 47dff7e
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?

reta · 2023-03-13T18:31:11Z

server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java

+            onPhaseFailure(
+                currentPhase,
+                "SearchTask was cancelled",
+                new TaskCancelledException(new OpenSearchRejectedExecutionException("cancelled task with reason: " + reason))


This layered exceptions wrapping does not look right, I think we should introduce dedicated exception (fe TaskBackpressureException or alike) to differentiate between cancellation modes.

opensearch-trigger-bot · 2023-07-03T15:22:57Z

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

PritLadani · 2023-07-05T15:21:45Z

Will get back to this. Open for other's contribution though.

opensearch-trigger-bot · 2023-08-06T15:20:57Z

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

opensearch-trigger-bot · 2023-08-14T15:20:53Z

This PR was closed because it has been stalled for 7 days with no activity.

kotwanikunal · 2023-09-14T18:45:10Z

Apologies. This PR was auto closed without reaching a resolution from the maintainers.
Re-opening to move it forward.
Thanks for your contributions to OpenSearch!

github-actions · 2023-09-14T18:47:22Z

Compatibility status:

Checks if related components are compatible with change 47dff7e

Incompatible components

Skipped components

Compatible components

github-actions · 2023-09-14T19:08:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/25635/
CommitID: 47dff7e
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?

ashking94 · 2023-12-26T08:04:21Z

@PritLadani is this being worked upon? Do tag the maintainers for closure on this if required.

PritLadani · 2023-12-26T13:16:31Z

@kkhatua can someone take a look at this, if not already addressed?

kaushalmahi12 · 2024-01-08T20:04:53Z

server/src/main/java/org/opensearch/tasks/TaskCancellationService.java

+
+    public static void throwTaskCancelledException(String reason) {
+        if (isRejection(reason)) {
+            throw new TaskCancelledException(new OpenSearchRejectedExecutionException("cancelled task with reason: " + reason));


Can't we just throw OpenSearchRejectedExecutionService if task is rejected ?

stephen-crawford · 2024-03-29T20:31:48Z

Hi @PritLadani, it seems like progress on this PR has stopped for a couple months. I recommend we close this for the time being to make sure we can keep track on everything going on. @peternied could you help by closing this PR for now?

Prit please feel free to re-open should you take up further work here.

peternied · 2024-03-29T21:37:30Z

@scrawfor99 Thank for pulling me in, but I don't have much context on this change.

@gbbafna @andrross would you might taking a look at this PR and make a call on what should happen next?

opensearch-trigger-bot · 2024-05-05T15:20:40Z

This PR is stalled because it has been open for 30 days with no activity.

dblock · 2024-07-15T16:28:56Z

Closing. Please reopen if you can/want to finish it.

[Catch All Triage - 1, 2, 3, 4]

Cancel in-flight search tasks due to search backpressure with 429 sta…

08eb7b7

…tus code Signed-off-by: PritLadani <pritkladani@gmail.com>

gbbafna reviewed Mar 11, 2023

View reviewed changes

PritLadani added 2 commits March 11, 2023 19:52

Addressing UT failure and comments

dfaae2d

Signed-off-by: PritLadani <pritkladani@gmail.com>

Adding changelog entry

47dff7e

Signed-off-by: PritLadani <pritkladani@gmail.com>

reta reviewed Mar 13, 2023

View reviewed changes

PritLadani mentioned this pull request Mar 30, 2023

[DOC] Search backpressure SearchTask cancellation documentation opensearch-project/documentation-website#3172

Open

4 tasks

opensearch-trigger-bot bot added the stalled Issues that have stalled label Jul 3, 2023

opensearch-trigger-bot bot removed the stalled Issues that have stalled label Jul 6, 2023

opensearch-trigger-bot bot added the stalled Issues that have stalled label Aug 6, 2023

opensearch-trigger-bot bot closed this Aug 14, 2023

kotwanikunal reopened this Sep 14, 2023

kotwanikunal requested review from peternied, dbwiddis, sachinpkale, sohami and msfroh as code owners September 14, 2023 18:45

kaushalmahi12 reviewed Jan 10, 2024

View reviewed changes

opensearch-trigger-bot bot removed the stalled Issues that have stalled label Jan 15, 2024

opensearch-trigger-bot bot added the stalled Issues that have stalled label May 5, 2024

dblock closed this Jul 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel in-flight search tasks due to search backpressure with 429 sta… #6634

Cancel in-flight search tasks due to search backpressure with 429 sta… #6634

PritLadani commented Mar 11, 2023 •

edited

Loading

github-actions bot commented Mar 11, 2023

gbbafna Mar 11, 2023

gbbafna Mar 11, 2023

PritLadani Mar 11, 2023

gbbafna Mar 11, 2023

andrross Feb 13, 2024

gbbafna Mar 11, 2023

PritLadani Mar 11, 2023

github-actions bot commented Mar 11, 2023

github-actions bot commented Mar 11, 2023

reta Mar 13, 2023

opensearch-trigger-bot bot commented Jul 3, 2023

PritLadani commented Jul 5, 2023 •

edited

Loading

opensearch-trigger-bot bot commented Aug 6, 2023

opensearch-trigger-bot bot commented Aug 14, 2023

kotwanikunal commented Sep 14, 2023

github-actions bot commented Sep 14, 2023

github-actions bot commented Sep 14, 2023

ashking94 commented Dec 26, 2023

PritLadani commented Dec 26, 2023

kaushalmahi12 Jan 8, 2024

stephen-crawford commented Mar 29, 2024

peternied commented Mar 29, 2024

opensearch-trigger-bot bot commented May 5, 2024

dblock commented Jul 15, 2024

Cancel in-flight search tasks due to search backpressure with 429 sta… #6634

Cancel in-flight search tasks due to search backpressure with 429 sta… #6634

Conversation

PritLadani commented Mar 11, 2023 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Mar 11, 2023

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 11, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 11, 2023

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Jul 3, 2023

PritLadani commented Jul 5, 2023 • edited Loading

opensearch-trigger-bot bot commented Aug 6, 2023

opensearch-trigger-bot bot commented Aug 14, 2023

kotwanikunal commented Sep 14, 2023

github-actions bot commented Sep 14, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Sep 14, 2023

Gradle Check (Jenkins) Run Completed with:

ashking94 commented Dec 26, 2023

PritLadani commented Dec 26, 2023

Choose a reason for hiding this comment

stephen-crawford commented Mar 29, 2024

peternied commented Mar 29, 2024

opensearch-trigger-bot bot commented May 5, 2024

dblock commented Jul 15, 2024

PritLadani commented Mar 11, 2023 •

edited

Loading

PritLadani commented Jul 5, 2023 •

edited

Loading