Repository navigation
Conversation
Co-Authored-By: Bee Klimt <bklimt@launchdarkly.com>
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
This branch has not been deployed
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
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.
Requirements
Related issues
SDK-2987 (part of SDK-2935). Spec: sdk-specs#251 (DATASYSTEM v2 1.6). Reference: cpp-sdks#631.
Describe the solution you've provided
Makes the server SDK's FDv1 fallback directive TTL-based. Fallback no longer lasts forever, and the SDK goes back to FDv2 when the TTL is up.
X-LD-FD-Fallback-TTL(polling and streaming) and fromgoodbye.protocolFallbackTTL. If a goodbye hasprotocolFallbackTTL, that counts as a directive by itself.FDv1FallbackDirective. The oldboolean fdv1Fallbackfactories onFDv2SourceResultstill work (truemeans the jittered default).Implementation details
FDv2DataSourcerun thread, and only results from the active synchronizer get applied. When no synchronizer replaces the active one,SourceManager.getNextAvailableSynchronizerAndSetActivenow closes it. Without this, the FDv2 synchronizer that sent the directive would stay connected while the SDK waits out the TTL with no FDv1 fallback.FDv1FallbackExpiryConditionis a new condition type. While fallback is engaged, it is the only condition on the FDv1 synchronizer, so the normal fallback/recovery conditions don't apply. It resets its timer whenever it sees a result that carries a directive.start()stays pending until FDv2 recovers instead of finishing withfalse, soLDClientstartup waits up tostartWait. If there are no FDv2 synchronizers to go back to, the FDv1 synchronizer just keeps running.Describe alternatives you've considered
Parsing
protocolFallbackTTLinlib/shared/internal'sGoodbye/FDv2ActionGoodbye. This PR deliberately leavesGoodbye.javaunchanged. The server SDK depends on the releasedlaunchdarkly-java-sdk-internal(1.12.2), so a change there would need its own internal release and then a bump PR before the server could use it. Instead, the server readsprotocolFallbackTTLstraight from the goodbye event's JSON (FDv1FallbackDirectives.fromGoodbye). That parsing can move intoGoodbyein a later change if reviewers want it.Additional context
No contract test changes, because the sdk-test-harness has no fallback-TTL coverage yet.
Link to Devin session: https://app.devin.ai/sessions/cdc21935a71a406bbe1b22dba462357c
Open in Devin Desktop: https://app.devin.ai/desktop/session/cdc21935a71a406bbe1b22dba462357c?variant=devin
Requested by: @beekld