Use "full jitter" & updated base delay for STANDARD retry mode defaults #2648
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Motivation and Context
The STANDARD retry mode has standardized behavior across the SDKs. This
includes using "full jitter" backoff (i.e., 0-100% of the computed
delay) for both normal and throttling errors. The standard behavior also
uses a base delay of 1 second.
## Modifications
* Update BackoffStrategy's `defaultStrategy` and
`defaultThrottlingStrategy` methods to accept a `RetryMode`. The
`RetryMode` will be used to resolve the default jitter strategy ("full"
vs. "equal") as well as the base delay (the max backoff time does not
vary between modes).
* Define additional constants in SdkDefaultRetrySetting to support the
update base delay values. (Note: This includes backwards-incompatible
changes but the class is annotated @SdkInternalApi.)
* Update RetryPolicy to use the new methods.
* Add tests to RetryPolicyTest to ensure that it is resolving to the
correct strategy and values.
* Add new unit test class for FullJitterBackoffStrategy. This confirms
that full jitter is consistent with the standard retry behavior.
* Update FullJitterBackoffStrategy to add 1 ms to the random result.
This both allows us to reach our maximum value (since the random
parameter is exclusive) and ensures we always have a delay of at least 1
ms (which is consistent with BackoffStrategy.none()'s behavior).
zoewangg
approved these changes
Aug 10, 2021
| @@ -0,0 +1,157 @@ | |||
| package software.amazon.awssdk.core.retry.backoff; | |||
| import software.amazon.awssdk.core.retry.RetryPolicyContext; | ||
|
|
||
| @RunWith(Parameterized.class) | ||
| public class FullJitterBackoffStrategyTest { |
added 2 commits
August 10, 2021 10:56
## Motivation and Context
The STANDARD retry mode has standardized behavior across the SDKs. This
includes using "full jitter" backoff (i.e., 0-100% of the computed
delay) for both normal and throttling errors. The standard behavior also
uses a base delay of 1 second.
## Modifications
* Update BackoffStrategy's `defaultStrategy` and
`defaultThrottlingStrategy` methods to accept a `RetryMode`. The
`RetryMode` will be used to resolve the default jitter strategy ("full"
vs. "equal") as well as the base delay (the max backoff time does not
vary between modes).
* Define additional constants in SdkDefaultRetrySetting to support the
update base delay values. (Note: This includes backwards-incompatible
changes but the class is annotated @SdkInternalApi.)
* Update RetryPolicy to use the new methods.
* Add tests to RetryPolicyTest to ensure that it is resolving to the
correct strategy and values.
* Add new unit test class for FullJitterBackoffStrategy. This confirms
that full jitter is consistent with the standard retry behavior.
* Update FullJitterBackoffStrategy to add 1 ms to the random result.
This both allows us to reach our maximum value (since the random
parameter is exclusive) and ensures we always have a delay of at least 1
ms (which is consistent with BackoffStrategy.none()'s behavior).
## Motivation and Context
The STANDARD retry mode has standardized behavior across the SDKs. This
includes using "full jitter" backoff (i.e., 0-100% of the computed
delay) for both normal and throttling errors. The standard behavior also
uses a base delay of 1 second.
## Modifications
* Update BackoffStrategy's `defaultStrategy` and
`defaultThrottlingStrategy` methods to accept a `RetryMode`. The
`RetryMode` will be used to resolve the default jitter strategy ("full"
vs. "equal") as well as the base delay (the max backoff time does not
vary between modes).
* Define additional constants in SdkDefaultRetrySetting to support the
update base delay values. (Note: This includes backwards-incompatible
changes but the class is annotated @SdkInternalApi.)
* Update RetryPolicy to use the new methods.
* Add tests to RetryPolicyTest to ensure that it is resolving to the
correct strategy and values.
* Add new unit test class for FullJitterBackoffStrategy. This confirms
that full jitter is consistent with the standard retry behavior.
* Update FullJitterBackoffStrategy to add 1 ms to the random result.
This both allows us to reach our maximum value (since the random
parameter is exclusive) and ensures we always have a delay of at least 1
ms (which is consistent with BackoffStrategy.none()'s behavior).
cenedhryn
approved these changes
Aug 10, 2021
## Motivation and Context
The STANDARD retry mode has standardized behavior across the SDKs. This
includes using "full jitter" backoff (i.e., 0-100% of the computed
delay) for both normal and throttling errors. The standard behavior also
uses a base delay of 1 second.
## Modifications
* Update BackoffStrategy's `defaultStrategy` and
`defaultThrottlingStrategy` methods to accept a `RetryMode`. The
`RetryMode` will be used to resolve the default jitter strategy ("full"
vs. "equal") as well as the base delay (the max backoff time does not
vary between modes).
* Define additional constants in SdkDefaultRetrySetting to support the
update base delay values. (Note: This includes backwards-incompatible
changes but the class is annotated @SdkInternalApi.)
* Update RetryPolicy to use the new methods.
* Add tests to RetryPolicyTest to ensure that it is resolving to the
correct strategy and values.
* Add new unit test class for FullJitterBackoffStrategy. This confirms
that full jitter is consistent with the standard retry behavior.
* Update FullJitterBackoffStrategy to add 1 ms to the random result.
This both allows us to reach our maximum value (since the random
parameter is exclusive) and ensures we always have a delay of at least 1
ms (which is consistent with BackoffStrategy.none()'s behavior).
|
Kudos, SonarCloud Quality Gate passed! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
The STANDARD retry mode has standardized behavior across the SDKs. This
includes using "full jitter" backoff (i.e., 0-100% of the computed
delay) for both normal and throttling errors. The standard behavior also
uses a base delay of 1 second.
Modifications
defaultStrategyanddefaultThrottlingStrategymethods to accept aRetryMode. TheRetryModewill be used to resolve the default jitter strategy ("full"vs. "equal") as well as the base delay (the max backoff time does not
vary between modes).
update base delay values. (Note: This includes backwards-incompatible
changes but the class is annotated @SdkInternalApi.)
correct strategy and values.
that full jitter is consistent with the standard retry behavior.
This both allows us to reach our maximum value (since the random
parameter is exclusive) and ensures we always have a delay of at least 1
ms (which is consistent with BackoffStrategy.none()'s behavior).
Types of changes
Checklist
mvn installsucceedsLicense