Skip to content

fix: Clamp the server FDv1 fallback TTL to (0, 1 hour] and jitter the default - #631

Open
beekld wants to merge 2 commits into
mainfrom
devin/1792429857-fdv1-fallback-ttl-clamp
Open

beekld wants to merge 2 commits into
mainfrom
devin/1792429857-fdv1-fallback-ttl-clamp

Conversation

@beekld

@beekld beekld commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This brings the server SDK FDv1 fallback directive in line with the updated spec (DATASYSTEM v2 1.6 / CSFDV2 8.3, sdk-specs#251): a fallback is never indefinite. Part of SDK-2982 / SDK-2935.

Before this change, X-LD-FD-Fallback-TTL / goodbye.protocolFallbackTTL were used verbatim, so 0 meant "stay on FDv1 forever": ScheduleFDv2RetryLocked returned early. There was no upper bound, and the 1h default for a missing TTL had no jitter.

FDv1FallbackDirective now matches the client SDK's API (libs/client-sdk/src/data_sources/fdv2/fdv2_source_result.*):

FDv1FallbackDirective::DefaultTtl();               // 1h - uniform[0, 30min]
FDv1FallbackDirective::FromServiceTtl(seconds);    // (0, 1h] used as-is, else DefaultTtl()
FDv1FallbackDirective::FromServiceTtl(string_view) // malformed -> DefaultTtl()
  • Header parsing in streaming and polling, and goodbye handling, now go through these factories. ParseTtl and the ttl = kDefaultTtl member default are removed. Both are internal, not public API.
  • FDv2DataSystem::ScheduleFDv2RetryLocked now always schedules FDv2 recovery. It already ran when no FDv1 synchronizer was configured.

Tests:

  • New fdv2_source_result_test.cpp, mirroring the client tests.
  • Header and goodbye tests for out-of-range TTLs.
  • A data-system test that FDv2 recovers after the TTL when no FDv1 adapter is configured.
  • Data-system tests that used ttl{0} to mean "no retry" now use a 1h TTL. They drain ready handlers (RunReadyHandlers, an ioc.poll() loop) instead of ioc.run(), which would now wait on the pending recovery timer.

The client SDK isn't touched because it already follows these semantics. The Windows jobs fail on main too, because Chocolatey can't find boost-msvc-14.3 1.87.0.

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


Note

Overview
Aligns the server SDK FDv1 fallback directive with the updated FDv2 spec: fallback is never indefinite, service TTLs are bounded, and the default retry delay is jittered.

FDv1FallbackDirective moves TTL construction into fdv2_source_result.cpp with DefaultTtl() (1h minus uniform jitter in [0, 30min]), FromServiceTtl(seconds) (uses values in (0, 1h] as-is; otherwise jittered default), and FromServiceTtl(string_view) for X-LD-FD-Fallback-TTL. Inline ParseTtl and the struct’s implicit default TTL are removed.

FDv2 polling, streaming, and goodbye paths now build directives through those factories instead of ad hoc parsing. FDv2DataSystem::ScheduleFDv2RetryLocked always schedules FDv2 recovery (the TTL=0 early return is gone), so even without an FDv1 adapter the SDK will retry FDv2 after the directive expires.

Tests add fdv2_source_result_test.cpp, cover out-of-range/malformed TTLs, assert recovery when no FDv1 synchronizer is configured, and use RunReadyHandlers where tests must not block on the recovery timer.

Reviewed by Cursor Bugbot for commit f0602e0. Bugbot is set up for automated code reviews on this repo. Configure here.

… default

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

…y timer is pending

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant