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.
Makes the server SDK's FDv1 fallback directive TTL-based and never permanent, per sdk-specs#251 (DATASYSTEM v2 1.6 / CSFDV2 8.3).
Closes SDK-2988, part of SDK-2935. Reference: cpp-sdks#631.
X-LD-FD-Fallback-TTLandgoodbye.protocolFallbackTTLare now read. A TTL in (0, 3600]s is used exactly as sent. A missing or invalid TTL becomes a 1 hour default jittered into [30min, 1h].protocolFallbackTTLis now treated as an FDv1 fallback directive (streaming and polling).Interruptedwith the original error, instead ofOff.Requirements
Implementation details
Before
FDv1FallbackActionAppliercalledBlockAll(FDv2), and nothing ever undid it. With no FDv1 synchronizer configured, the outer composite ran out of sources and the data system wentOff. Neither TTL source was read, and a goodbye never started a fallback.Describe the solution you've provided
FDv1FallbackTtl):Default()returns 1h minus uniform [0, 30min].FromServiceTtl(seconds)returns the TTL as-is when it is in (0, 3600], and the default otherwise.FromHeaders(...)requires whole seconds; anything else gets the default.Goodbyepayload now parsesprotocolFallbackTTL. If the field is present but not a number, it still counts as a directive and uses the default TTL.FDv2ActionGoodbye.FDv1FallbackTtlis null when there is no directive.ErrorInfo.FDv1FallbackTtlandChangeSet.FDv1FallbackTtl, so the public API is unchanged.DirectedFallbackRecovery, one per FDv2 data source):ICompositeSourceActionable.UnblockAll()restores the original entries, thenGoToFirst/StartCurrentre-run the full initializer + synchronizer flow (spec 1.6.4.2 allows this).GoToFirstdisconnects and disposes the FDv1 source before building the FDv2 source.Describe alternatives you've considered
Off.Deliberately left unchanged
X-LD-FD-Fallback, so this case can't happen today.Start()result with no FDv1 synchronizer. When the directive arrives before any data and no FDv1 synchronizer is configured,Start()still resolvesfalse, as before, andInitializationTrackerstays terminal. A later recovery that brings data doesn't flipLdClient.Initialized, although evaluations still use the store.directive without FDv1 fallback configured halts the data systemanddirected fallback is terminaltests should still pass, because a directive with no TTL now defaults to 30–60min, which is longer than those tests watch. Their names and comments describe the old semantics, though, and should be updated in the harness.Tests
FDv1FallbackTtlTest: valid TTLs come back unchanged; invalid ones give a default in [30min, 1h].Link to Devin session: https://app.devin.ai/sessions/e8d6f6e762ae490db6101c46612b1306
Open in Devin Desktop: https://app.devin.ai/desktop/session/e8d6f6e762ae490db6101c46612b1306?variant=devin
Requested by: @beekld