-
Notifications
You must be signed in to change notification settings - Fork 245
DRIVERS-3427 Phase 1 rollout: configurable retries, retargeting knob #1917
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
base: master
Are you sure you want to change the base?
Changes from all commits
0f305d8
72cc464
ab34435
b6b29ae
dac8a50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -122,7 +122,7 @@ rules: | |||||||||||
| 2. A retry attempt will only be permitted if: | ||||||||||||
| 1. The error is a retryable overload error. | ||||||||||||
| 2. We have not reached `MAX_RETRIES`. | ||||||||||||
| - The value of `MAX_RETRIES` is 5 and non-configurable. | ||||||||||||
| - The default value of `MAX_RETRIES` is 2. Drivers MUST expose this as a configurable option `maxRetries`. | ||||||||||||
| - This intentionally changes the behavior of CSOT which otherwise would retry an unlimited number of times within | ||||||||||||
| the timeout to avoid retry storms. | ||||||||||||
| 3. (CSOT-only): There is still time for a retry attempt according to the | ||||||||||||
|
|
@@ -142,9 +142,10 @@ rules: | |||||||||||
| - `MAX_BACKOFF` is 10000ms. | ||||||||||||
| - This results in delays of 100ms, 200ms, 400ms, 800ms, and 1600ms before accounting for jitter. | ||||||||||||
| 4. If the request is eligible for retry (as outlined in step 2 above and step 4 in the | ||||||||||||
| [adaptive retry requirements](client-backpressure.md#adaptive-retry-requirements) below), the client MUST add the | ||||||||||||
| previously used server's address to the list of deprioritized server addresses for | ||||||||||||
| [server selection](../server-selection/server-selection.md). | ||||||||||||
| [adaptive retry requirements](client-backpressure.md#adaptive-retry-requirements) below) and | ||||||||||||
| `enableOverloadRetargeting` is enabled, the client MUST add the previously used server's address to the list of | ||||||||||||
| deprioritized server addresses for [server selection](../server-selection/server-selection.md). Drivers MUST expose | ||||||||||||
| this as a configurable boolean option named `enableOverloadRetargeting` defaulting to `false`. | ||||||||||||
|
Comment on lines
+147
to
+148
|
||||||||||||
| deprioritized server addresses for [server selection](../server-selection/server-selection.md). Drivers MUST expose | |
| this as a configurable boolean option named `enableOverloadRetargeting` defaulting to `false`. | |
| deprioritized server addresses for [server selection](../server-selection/server-selection.md). If | |
| `enableOverloadRetargeting` is disabled, the client MUST NOT add the previously used server's address to this list. | |
| Drivers MUST expose this as a configurable boolean option named `enableOverloadRetargeting` defaulting to `false`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description mentions a new
maxAdaptiveRetriesdriver option, but this spec text requires drivers to exposeMAX_RETRIESasmaxRetries. Please align the option name between the PR description and the spec (and ensure the chosen name is used consistently throughout the spec).