-
Notifications
You must be signed in to change notification settings - Fork 12
feat(server): RETRY-spec conformance in FDv1 streaming and polling #200
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
Open
tanderson-ld
wants to merge
2
commits into
main
Choose a base branch
from
ta/SDK-2789/retry-conformance-work
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
147 changes: 147 additions & 0 deletions
147
lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/PollingStrategy.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| package com.launchdarkly.sdk.server; | ||
|
|
||
| import com.launchdarkly.sdk.internal.http.FailureClass; | ||
|
|
||
| import java.time.Duration; | ||
| import java.util.Random; | ||
|
|
||
| /** | ||
| * Retry-timing state machine for the polling data source. Selects a per-attempt | ||
| * delay based on prior outcomes: | ||
| * <ul> | ||
| * <li>Normal regime: successive attempts wait {@code pollInterval}. No backoff | ||
| * is applied because {@code initialDelay} and {@code maxDelay} both equal | ||
| * {@code pollInterval}.</li> | ||
| * <li>Extended regime: entered on an {@link FailureClass#UNEXPECTED} failure. | ||
| * Waits start at {@code extendedInitialInterval} (floored at | ||
| * {@code pollInterval}) and double each attempt, clamped to | ||
| * {@link #EXTENDED_MAX_DELAY}.</li> | ||
| * <li>Healthy-op reset: two consecutive successful polls return the strategy | ||
| * to the normal regime.</li> | ||
| * </ul> | ||
| * <p> | ||
| * The formula input {@code n} in {@code T = initialDelay * 2^(n-1)} resets to | ||
| * zero whenever the delay bounds change (regime transition), so the first | ||
| * attempt in the new regime uses the new initial delay directly. | ||
| * <p> | ||
| * All state is owned by the polling loop's own thread (currently the shared | ||
| * ScheduledExecutorService in {@link PollingProcessor}). No external synchronization | ||
| * is required as long as this invariant holds. | ||
| */ | ||
| final class PollingStrategy { | ||
| static final Duration EXTENDED_MAX_DELAY = Duration.ofHours(1); | ||
|
|
||
| private final Duration normalInterval; | ||
| private final Duration extendedInitialInterval; | ||
| private final Random rng; | ||
|
|
||
| private int n; | ||
| private boolean priorPollWasSuccessful; | ||
| private boolean inExtended; | ||
| private Duration initialDelay; | ||
| private Duration maxDelay; | ||
|
|
||
| PollingStrategy(Duration normalInterval, Duration extendedInitialInterval) { | ||
| this(normalInterval, extendedInitialInterval, new Random()); | ||
| } | ||
|
|
||
| // Visible for testing; deterministic seed injectable so jitter is reproducible. | ||
| PollingStrategy(Duration normalInterval, Duration extendedInitialInterval, Random rng) { | ||
| this.normalInterval = normalInterval; | ||
| this.extendedInitialInterval = extendedInitialInterval; | ||
| this.rng = rng; | ||
| // Normal regime at construction: both initialDelay and maxDelay equal the | ||
| // customer-configured pollInterval (there's no backoff in the normal | ||
| // regime — successive normal-failure retries stay at pollInterval). | ||
| this.initialDelay = normalInterval; | ||
| this.maxDelay = normalInterval; | ||
| } | ||
|
|
||
| /** | ||
| * Advance state after a poll failure. Returns {@code true} exactly once per | ||
| * transition from the normal regime into the extended regime; the caller | ||
| * can use the return value to emit an operator-visible log at the moment of | ||
| * transition without re-firing on every subsequent UNEXPECTED failure while | ||
| * already in extended regime. | ||
| * <p> | ||
| * On the transition, set {@code n = 1} and swap in the extended bounds, so | ||
| * the first extended wait uses {@code extendedInitialInterval} directly | ||
| * (the "reset n when delays change" invariant). On any other failure, | ||
| * increment n so the delay doubles. | ||
| * <p> | ||
| * Extended-regime bounds are floored at the customer-configured | ||
| * {@code pollInterval} — the wait never drops below that. | ||
| */ | ||
| boolean onFailure(FailureClass failureClass) { | ||
| this.priorPollWasSuccessful = false; | ||
| if (failureClass == FailureClass.UNEXPECTED && !this.inExtended) { | ||
| this.inExtended = true; | ||
| this.n = 1; | ||
| this.initialDelay = extendedInitialInterval; | ||
| if (this.initialDelay.compareTo(normalInterval) < 0) { | ||
| this.initialDelay = normalInterval; | ||
| } | ||
| this.maxDelay = EXTENDED_MAX_DELAY; | ||
| if (this.maxDelay.compareTo(normalInterval) < 0) { | ||
| this.maxDelay = normalInterval; | ||
| } | ||
| return true; | ||
| } | ||
| this.n++; | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Advance state after a poll success. After two successes in a row, n resets | ||
| * to zero and delay bounds revert to the normal regime. A single success | ||
| * sets a "prior succeeded" flag; any intervening failure clears it. | ||
| * <p> | ||
| * The reset also clears {@code inExtended} so a subsequent UNEXPECTED | ||
| * failure re-transitions into the extended regime (with the transition | ||
| * detected exactly once, per {@link #onFailure(FailureClass)}'s contract). | ||
| */ | ||
| void onSuccess() { | ||
| if (this.priorPollWasSuccessful) { | ||
| this.n = 0; | ||
| this.inExtended = false; | ||
| this.initialDelay = normalInterval; | ||
| this.maxDelay = normalInterval; | ||
| } | ||
| this.priorPollWasSuccessful = true; | ||
| } | ||
|
|
||
| /** | ||
| * Compute the delay before the next poll attempt: | ||
| * {@code T = initialDelay * 2^(n-1)}, clamped to {@code maxDelay}. Jitter | ||
| * {@code J} is uniform in {@code [0, T/2]}. Final wait is | ||
| * {@code max(pollInterval, T - J)} — the wait never drops below the | ||
| * customer-configured {@code pollInterval}. | ||
| */ | ||
| Duration nextWait() { | ||
| if (this.n <= 0) { | ||
| return normalInterval; | ||
| } | ||
| long initialMs = initialDelay.toMillis(); | ||
| long maxMs = maxDelay.toMillis(); | ||
| double factor = Math.pow(2, this.n - 1); | ||
| long tMs = (long) Math.min(initialMs * factor, (double) maxMs); | ||
| long jitterMs = 0; | ||
| long halfT = tMs / 2; | ||
| if (halfT > 0) { | ||
| jitterMs = (rng.nextLong() % halfT + halfT) % halfT; | ||
| } | ||
| long waitMs = tMs - jitterMs; | ||
| long floorMs = normalInterval.toMillis(); | ||
| if (waitMs < floorMs) { | ||
| waitMs = floorMs; | ||
| } | ||
| return Duration.ofMillis(waitMs); | ||
| } | ||
|
|
||
| // Accessors for observability / testing. | ||
|
|
||
| int getN() { return n; } | ||
| Duration getInitialDelay() { return initialDelay; } | ||
| Duration getMaxDelay() { return maxDelay; } | ||
| boolean getPriorPollWasSuccessful() { return priorPollWasSuccessful; } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Extended wait after poll success
Medium Severity
After a successful poll while still in the extended regime,
onSuccessleavesnunchanged, sonextWaitkeeps returning the large extended backoff. The confirming second success can be delayed by up to an hour, which stalls recovery long after the data source is healthy again and conflicts with returning to the configured poll cadence.Reviewed by Cursor Bugbot for commit 9394a65. Configure here.