Add basic thorttler/exponential backoff policy for retry/Defination o… #3527

dhwanilpatel · 2022-06-07T10:05:25Z

Add basic thorttler/exponential backoff policy for retry/Defination of throttling exception

Signed-off-by: Dhwanil Patel dhwanip@amazon.com

Description

This is one of multiple PR planned for master task throttling. In this PR we are introducing couple of new classes required for upcoming PRs.

New class/Method introduced in this PR:
Throttler : Base class for Throttling logic. It provides throttling functionality over multiple keys.
ExponentialBackOffPolicy : It provides exponential backoff between retries until it reaches maxDelay. It uses equal jitter scheme as it is being used for throttled exceptions. It will make random distribution and also guarantees a minimum delay.
AdjustableSemaphore: AdjustableSemaphore is Extended Semphore where we can change maxPermits.
MasterTaskThrottlingException : new Throttling Exception which will be thrown from master node whenever it throttles any tasks.

Issues Resolved

Relates to : #479

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

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.

…f throttling exception Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

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

opensearch-ci-bot · 2022-06-07T10:29:13Z

❌ Gradle Check failure 95e55e5
Log 5817

Reports 5817

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

server/src/main/java/org/opensearch/cluster/service/Throttler.java

server/src/main/java/org/opensearch/common/AdjustableSemaphore.java

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

server/src/main/java/org/opensearch/common/AdjustableSemaphore.java

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

opensearch-ci-bot · 2022-06-17T15:10:23Z

✅ Gradle Check success bc32248
Log 6106

Reports 6106

Bukhtawar · 2022-06-17T15:22:37Z

server/src/main/java/org/opensearch/cluster/service/Throttler.java

+        for (T key : semaphores.keySet()) {
+            semaphores.put(key, new AdjustableSemaphore(semaphores.get(key).getMaxPermits(), true));
+        }
+    }


I think we should have the constructor intialize the semaphore with the desired permits

Can you please elaborate more on it? We are passing the desired permits in constructor of Semaphore itslef.

server/src/main/java/org/opensearch/cluster/service/Throttler.java

Bukhtawar · 2022-06-17T15:29:39Z

server/src/main/java/org/opensearch/action/bulk/BackoffPolicy.java

+        private ExponentialEqualJitterBackoffIterator(int baseDelay, int maxDelay) {
+            this.baseDelay = baseDelay;
+            this.maxDelay = maxDelay;
+        }


Should we have max retry in the constructor?

No we dont want max retry for this policy. This policy will eventually perform infinite retries with exponential backoff.

server/src/main/java/org/opensearch/action/bulk/BackoffPolicy.java

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

server/src/main/java/org/opensearch/action/bulk/BackoffPolicy.java

server/src/main/java/org/opensearch/cluster/service/Throttler.java

server/src/test/java/org/opensearch/cluster/service/ThrottlerTests.java

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

github-actions · 2022-07-04T13:47:45Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/127/
CommitID: 0100d65

server/src/main/java/org/opensearch/cluster/service/Throttler.java

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

github-actions · 2022-07-05T11:08:17Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/134/
CommitID: 0c9bea5022b48b3d76e84ac67b4589d9760e404f

github-actions · 2022-07-05T11:32:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/135/
CommitID: 049c3de

server/src/main/java/org/opensearch/cluster/service/Throttler.java

…' into throttling-pr-1 Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

github-actions · 2022-07-11T10:14:31Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/372/
CommitID: cbaabb1

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

dhwanilpatel · 2022-07-11T14:02:32Z

@dblock / @reta My latest build(https://build.ci.opensearch.org/job/gradle-check/372/) was failing due to this bug. (#3838).

Since the fix is merged in main branch, I am rebasing my PR against latest main branch.

github-actions · 2022-07-11T14:28:32Z

Gradle Check (Jenkins) Run Completed with:

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

dhwanilpatel · 2022-07-11T14:31:33Z

@dblock / @reta Since build has passed, can you please help rebase the feature branch (https://github.com/opensearch-project/OpenSearch/tree/feature/master-task-throttling) with latest fix (since it is behind couple commit from main), and then merge this PR into feature branch.

Thanks in Advance!!

reta · 2022-07-11T14:59:04Z

@dblock / @reta Since build has passed, can you please help rebase the feature branch (https://github.com/opensearch-project/OpenSearch/tree/feature/master-task-throttling) with latest fix (since it is behind couple commit from main), and then merge this PR into feature branch.

@dhwanilpatel Sadly I cannot push to origin branches, we could open the pull request against it to bring latest changes from main, for example.

dhwanilpatel · 2022-07-11T15:25:30Z

Sure @reta, I have created PR for rebasing feature branch with main to pull the latest changes. Please check it and merge.

#3843

dblock · 2022-07-11T22:16:17Z

Did I hit merge on the wrong PR? I was supposed to merge #3843?

…nation o… (#3527)" This reverts commit b8db5bf. Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>

…nation o… (#3527)" (#3852) This reverts commit b8db5bf. Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>

dblock · 2022-07-11T23:15:25Z

@dhwanilpatel Please reopen a PR onto feature/master-task-throttling. In general I think we don't need to rebase feature branches, you can just make merge commits as you see fit.

dhwanilpatel · 2022-07-12T05:28:26Z

@reta / @dblock Created the PR against feature/master-task-throttling.

New PR: #3856.

Basic Throttler Framework / Exponential Basic back off policy. Add basic thorttler/exponential backoff policy for retry/Defination o… #3527 Changes required in Master node to perform throttling. Master node changes for master task throttling #3882 Changes required in Data node to perform retry on throttling. Data node changes for master task throttling #4204 Provide support for all task type in throttling framework. Onboarding of few task types to throttling #4542 Integration Tests (Fix timeout exception and Add Integ test for Master task throttling #4588 Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

…t#4986) Basic Throttler Framework / Exponential Basic back off policy. Add basic thorttler/exponential backoff policy for retry/Defination o… opensearch-project#3527 Changes required in Master node to perform throttling. Master node changes for master task throttling opensearch-project#3882 Changes required in Data node to perform retry on throttling. Data node changes for master task throttling opensearch-project#4204 Provide support for all task type in throttling framework. Onboarding of few task types to throttling opensearch-project#4542 Integration Tests (Fix timeout exception and Add Integ test for Master task throttling opensearch-project#4588 Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

Basic Throttler Framework / Exponential Basic back off policy. Add basic thorttler/exponential backoff policy for retry/Defination o… #3527 Changes required in Master node to perform throttling. Master node changes for master task throttling #3882 Changes required in Data node to perform retry on throttling. Data node changes for master task throttling #4204 Provide support for all task type in throttling framework. Onboarding of few task types to throttling #4542 Integration Tests (Fix timeout exception and Add Integ test for Master task throttling #4588 Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

Add basic thorttler/exponential backoff policy for retry/Defination o…

95e55e5

…f throttling exception Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

dhwanilpatel requested review from a team and reta as code owners June 7, 2022 10:05

dhwanilpatel mentioned this pull request Jun 7, 2022

Cluster Manager Task Throttling #479

Closed

dhwanilpatel commented Jun 7, 2022

View reviewed changes

server/src/main/java/org/opensearch/OpenSearchException.java Outdated Show resolved Hide resolved

gbbafna reviewed Jun 14, 2022

View reviewed changes

server/src/main/java/org/opensearch/OpenSearchException.java Outdated Show resolved Hide resolved

server/src/main/java/org/opensearch/cluster/service/Throttler.java Show resolved Hide resolved

server/src/main/java/org/opensearch/common/AdjustableSemaphore.java Show resolved Hide resolved

Bukhtawar reviewed Jun 14, 2022

View reviewed changes

server/src/main/java/org/opensearch/OpenSearchException.java Outdated Show resolved Hide resolved

server/src/main/java/org/opensearch/common/AdjustableSemaphore.java Show resolved Hide resolved

Incorporated comments

bc32248

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

Bukhtawar reviewed Jun 17, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/service/Throttler.java Outdated Show resolved Hide resolved

Bukhtawar reviewed Jun 17, 2022

View reviewed changes

server/src/main/java/org/opensearch/action/bulk/BackoffPolicy.java Show resolved Hide resolved

gbbafna reviewed Jun 20, 2022

View reviewed changes

server/src/main/java/org/opensearch/action/bulk/BackoffPolicy.java Outdated Show resolved Hide resolved

dhwanilpatel added 2 commits June 29, 2022 12:24

Incorporated minor comments

a24817b

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

Merge remote-tracking branch 'upstream/main' into throttling-pr-1

da09a5e

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

gbbafna approved these changes Jun 29, 2022

View reviewed changes

shwetathareja reviewed Jul 4, 2022

View reviewed changes

Removed overall thorttlingEnabled flag from Throttler

0100d65

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

reta reviewed Jul 4, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/service/Throttler.java Outdated Show resolved Hide resolved

reta reviewed Jul 4, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/service/Throttler.java Outdated Show resolved Hide resolved

Corrected Java doc for Throttler

049c3de

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

dhwanilpatel force-pushed the throttling-pr-1 branch from 0c9bea5 to 049c3de Compare July 5, 2022 11:07

reta reviewed Jul 5, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/service/Throttler.java Outdated Show resolved Hide resolved

reta reviewed Jul 5, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/service/Throttler.java Outdated Show resolved Hide resolved

reta reviewed Jul 5, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/service/Throttler.java Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/feature/master-task-throttling…

cbaabb1

…' into throttling-pr-1 Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

Merge remote-tracking branch 'upstream/main' into throttling-pr-1

f515d32

Signed-off-by: Dhwanil Patel <dhwanip@amazon.com>

dblock merged commit b8db5bf into opensearch-project:main Jul 11, 2022

dblock mentioned this pull request Jul 11, 2022

Revert "Add basic thorttler/exponential backoff policy for retry/Defination o…" #3852

Merged

dblock added a commit that referenced this pull request Jul 11, 2022

Revert "Add basic thorttler/exponential backoff policy for retry/Defi…

58f3677

…nation o… (#3527)" This reverts commit b8db5bf. Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>

dblock added a commit that referenced this pull request Jul 11, 2022

Revert "Add basic thorttler/exponential backoff policy for retry/Defi…

f35b42f

…nation o… (#3527)" (#3852) This reverts commit b8db5bf. Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>

dhwanilpatel mentioned this pull request Jul 12, 2022

Add basic thorttler/exponential backoff policy for retry/Defination of throttling exception #3856

Merged

5 tasks

This was referenced Oct 31, 2022

[Feature]Cluster manager task throttling feature #4986

Merged

[Backport 2.x] Cluster Manager task throttling #5041

Merged

Cluster manager task throttling [DOC] opensearch-project/documentation-website#1792

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic thorttler/exponential backoff policy for retry/Defination o… #3527

Add basic thorttler/exponential backoff policy for retry/Defination o… #3527

dhwanilpatel commented Jun 7, 2022

opensearch-ci-bot commented Jun 7, 2022

opensearch-ci-bot commented Jun 17, 2022

Bukhtawar Jun 17, 2022

dhwanilpatel Jun 22, 2022

Bukhtawar Jun 17, 2022

dhwanilpatel Jun 22, 2022 •

edited

Loading

github-actions bot commented Jul 4, 2022

github-actions bot commented Jul 5, 2022

github-actions bot commented Jul 5, 2022

github-actions bot commented Jul 11, 2022

dhwanilpatel commented Jul 11, 2022

github-actions bot commented Jul 11, 2022

dhwanilpatel commented Jul 11, 2022

reta commented Jul 11, 2022

dhwanilpatel commented Jul 11, 2022

dblock commented Jul 11, 2022

dblock commented Jul 11, 2022

dhwanilpatel commented Jul 12, 2022

Add basic thorttler/exponential backoff policy for retry/Defination o… #3527

Add basic thorttler/exponential backoff policy for retry/Defination o… #3527

Conversation

dhwanilpatel commented Jun 7, 2022

Description

Issues Resolved

Check List

opensearch-ci-bot commented Jun 7, 2022

opensearch-ci-bot commented Jun 17, 2022

Bukhtawar Jun 17, 2022

Choose a reason for hiding this comment

dhwanilpatel Jun 22, 2022

Choose a reason for hiding this comment

Bukhtawar Jun 17, 2022

Choose a reason for hiding this comment

dhwanilpatel Jun 22, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 5, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 5, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 11, 2022

Gradle Check (Jenkins) Run Completed with:

dhwanilpatel commented Jul 11, 2022

github-actions bot commented Jul 11, 2022

Gradle Check (Jenkins) Run Completed with:

dhwanilpatel commented Jul 11, 2022

reta commented Jul 11, 2022

dhwanilpatel commented Jul 11, 2022

dblock commented Jul 11, 2022

dblock commented Jul 11, 2022

dhwanilpatel commented Jul 12, 2022

dhwanilpatel Jun 22, 2022 •

edited

Loading