Skip to content

fix: Make FDv1 fallback TTL-based and always recover to FDv2 - #542

Draft
beekld wants to merge 1 commit into
mainfrom
devin/1792430302-fdv1-fallback-ttl
Draft

beekld wants to merge 1 commit into
mainfrom
devin/1792430302-fdv1-fallback-ttl

Conversation

@beekld

@beekld beekld commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Makes the FDv1 fallback directive TTL-based and never terminal, per the SDK-2527 spec change (sdk-specs#251, DATASYSTEM v2 1.6 / CSFDV2 8.3).

Closes SDK-2989. Part of SDK-2935. Mirrors cpp-sdks#631.

  • The TTL comes from X-LD-FD-Fallback-TTL or goodbye.protocolFallbackTTL (seconds). Values in (0, 3600] are used as-is. Missing, malformed, zero, negative, or > 3600 gives the 1 hour default minus uniform [0, 1800]s of jitter.
  • A goodbye carrying protocolFallbackTTL now acts as a fallback directive, on both streaming and polling.
  • FDv2 / AsyncFDv2 keep the FDv2 synchronizer list, run FDv1 (if configured) for the TTL, then restart FDv2 from the first synchronizer.
  • With no FDv1 fallback synchronizer configured, the data source reports INTERRUPTED (not OFF) and FDv2 is retried after the TTL.

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions
  • Sync/async parity: matching changes made to async_* siblings (or N/A)
Implementation details

Describe the solution you've provided

  • ldclient/impl/util.py: fdv1_fallback_ttl() applies the TTL rules. fdv1_fallback_ttl_from_headers() reads both headers.
  • Update and Basis gain an optional fallback_ttl. Goodbye gains protocol_fallback_ttl.
  • Streaming: the latched Start-header directive now carries its TTL. A goodbye with protocolFallbackTTL emits an INTERRUPTED update with fallback_to_fdv1=True and halts the stream. A goodbye without it is still just logged.
  • Polling: with_goodbye_fallback() turns an in-band goodbye TTL into the same headers the existing fallback path already reads. A goodbye-only payload is no longer an error (it returns no changes).
  • Orchestration: _run_fdv1_fallback() runs the FDv1 synchronizer until the TTL expires, using a one-shot recovery timer in _consume_synchronizer_results, or just waits on the stop event when no FDv1 synchronizer is configured. Store writes still happen only on the coordinator thread/task, and each synchronizer is stopped before the next one starts, so FDv1 and FDv2 never write concurrently.

Describe alternatives you've considered

  • Carrying the polling goodbye TTL as a third element of the (ChangeSet, headers) result instead of folding it into headers. That would have changed every polling requester and the test mocks that build those tuples.

Additional context

  • Deliberately unchanged: a valid header TTL must be an integer string (for example "1.5" gets the default). Recovery restarts all configured FDv2 synchronizers, including any that were removed after a permanent failure before the fallback.
  • During initialization, INTERRUPTED is still reported as INITIALIZING by the existing status mapping.
  • No contract-test changes: as far as I could find, the sdk-test-harness has no fallback-TTL coverage.
  • Tests: unit tests for valid/invalid TTLs and the jitter bounds, header and goodbye TTL propagation (sync/async streaming and polling), and FDv2 recovery with and without an FDv1 synchronizer (sync and async). make lint passes, and so does the full pytest run with LD_SKIP_DATABASE_TESTS=1.

Link to Devin session: https://app.devin.ai/sessions/1439eb6230bf40f1bb73a3fd22612165
Open in Devin Desktop: https://app.devin.ai/desktop/session/1439eb6230bf40f1bb73a3fd22612165?variant=devin
Requested by: @beekld

Co-Authored-By: Bee Klimt <bklimt@launchdarkly.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant