Skip to content

fix: Use monotonic clocks for stream-init durations and FDv2 state windows - #537

Open
tanderson-ld wants to merge 1 commit into
mainfrom
tanderson/sdk-3105-monotonic-clock
Open

tanderson-ld wants to merge 1 commit into
mainfrom
tanderson/sdk-3105-monotonic-clock

Conversation

@tanderson-ld

@tanderson-ld tanderson-ld commented Oct 9, 2026 •

Copy link
Copy Markdown

Summary

Replaces wall-clock time with monotonic time in the two remaining places this SDK measures a duration or decides behavior from one: stream-init diagnostics and the FDv2 fallback/recovery windows. Wall clocks remain only for values reported outside the process or compared against externally supplied times. Implements SDK-3105.

A wall-clock step (NTP correction, manual clock set, container/VM time resync, the time correction when a frozen environment thaws) previously could cause:

  • Stream-init diagnostics (datasource/streaming.py, datasource/async_streaming.py): a forward step inflated the reported durationMillis by the step size; a backward step hit the elapsed >= 0 else 0 clamp and silently reported zero — a lost measurement either way.
  • FDv2 fallback/recovery verdicts (datasystem/fdv2_common.py, 60s/10s/300s windows): a forward step could make a just-interrupted source look an hour degraded and trigger an immediate synchronizer fallback; a backward step could suppress a legitimate one.

Two audit findings in this story's original scope — the wall-clock schedulers in repeating_task.py and aio/concurrency.py — were already resolved on main by the RepeatingTask/DelaySource rework that shipped in 9.18.0 (the wait is event-based with no clock arithmetic), so they appear here as this note rather than as code.

Changes

  • Impl::util.monotonic_seconds() (time.monotonic()) is the single monotonic seam.
  • Stream-init: the connection-attempt stamp is now monotonic and serves as both the duration base and the "attempt in flight" sentinel (explicit is not None, which also fixes a latent float-truthiness wart in the old guard). The reported timestamp stays wall-clock; the negative-value clamp is removed because a negative duration is now impossible.
  • FDv2 windows: the condition-check loop in each coordinator owns a small _StateAgeTracker — created per synchronizer session, so windows reset naturally on failover — which measures time-in-state monotonically from first observation. Its identity key is (state, since): a real transition changes since even when the state repeats (an interrupted→valid→interrupted flap between two 10-second checks resets the window), while a same-state error update preserves it (a stream of error reports cannot keep resetting the clock). since is used only for equality, never arithmetic. fallback_condition/recovery_condition take the measured seconds as a parameter and are pure, table-testable predicates. This observer-owned shape matches the Rust server SDK's FDv2 implementation, which builds its failover deadlines from Instant::now() inside the coordinator.
  • The status provider and the public DataSourceStatus are untouched — zero diff against main.

Deliberately unchanged (wall clock is correct): DataSourceStatus.since, DataSourceErrorInfo.time, event creationDates, diagnostic payload timestamps, debugEventsUntilDate comparisons, and big-segments staleness (cross-process stamp).

Behavioral notes

Not a breaking change — every modified component is Impl-internal; the public status surface is untouched; behavior under a stable clock is identical, with one bounded exception:

  1. Observation latency. Time-in-state is measured from when the check loop first sees a state, not from the transition itself, so a verdict can fire up to one 10-second check period later than before (material only for the 10-second INITIALIZING window, where worst-case detection moves from ~20s to ~30s). Always in the safe direction — defer, never spuriously trigger.
  2. Suspend semantics. time.monotonic() on Linux does not advance during system suspend, so these measurements now count process-visible time. Notably this costs nothing in AWS Lambda: a frozen execution environment pauses the entire microVM with no guest-visible suspend, so every in-process clock behaves identically there — while the thaw-time wall correction is exactly the kind of step this change defends against.
  3. Test suites that freeze time. Applications using freezegun (or similar) to drive SDK-internal timing should note these measurements no longer follow time.time(); freezegun does not patch time.monotonic() by default.

Known sibling, deliberately out of scope

ldclient/impl/datasourcev2/streaming.py and async_streaming.py carry the same wall-clock stream-init pattern (including a wall-clock future-start stamp from classify_stream_error). Those files postdate the originating audit; they'll be handled under a separate decision rather than folded in here.

Relationship to the Ruby change

Same change-class as launchdarkly/ruby-server-sdk#459. The FDv2 window fix intentionally uses a different shape than Ruby's (observer-owned tracker here vs. a provider-held stamp there); the Rust FDv2 implementation is the shared precedent for the approach used here.

Testing

Full suite passes (1766 passed, 371 skipped — database integrations). New test_fdv2_conditions.py covers the threshold truth table (strict-> boundaries pinned), the tracker's first-observation/flap/error-spam semantics, and a regression test proving a +1h wall step can no longer trigger a spurious fallback. Wall-step regression tests added to both streaming test suites use a forward step deliberately: the old code clamped negatives to zero, so only the forward direction distinguishes old from new behavior. mypy, isort, and pycodestyle clean.


Note

Overview
Replaces wall-clock timing with monotonic measurements for two internal SDK behaviors so NTP or manual clock jumps cannot skew diagnostics or failover decisions.

Stream-init diagnostics (sync and async streaming processors): connection-attempt duration is computed from monotonic_seconds() instead of time.time(), while reported timestamps stay wall-clock. The old negative-duration clamp is removed; retry backoff is excluded from measured latency by stamping the attempt start after the wait.

FDv2 fallback/recovery (fdv2_common, wired in sync/async coordinators): a per-session _StateAgeTracker measures how long the current data-source state has been observed on the monotonic timeline (keyed by (state, since) so error spam does not reset the window). fallback_condition / recovery_condition now take that measured seconds_in_state instead of time.time() - status.since.

Adds monotonic_seconds() in impl/util.py and regression tests for wall-clock steps and FDv2 thresholds.

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

…ndows

Wall-clock steps (NTP corrections, manual clock sets, VM/container time
resync, thaw-time corrections in frozen environments) could inflate the
stream-init durations reported in diagnostic events (or silently drop
them via the negative-value clamp), and could trigger or suppress the
FDv2 fallback/recovery verdicts by distorting the measured time in
state.

Stream-init durations are now measured on the monotonic clock, with the
wall clock kept only for the reported timestamp. The FDv2 coordinators
measure time in state with a small observer (_StateAgeTracker) owned by
the condition-check loop, keyed on (state, since) so flaps reset the
window and same-state error reports do not.

Note for applications that use freezegun or similar to drive
SDK-internal timing: these measurements no longer follow time.time().

SDK-3105
@tanderson-ld
tanderson-ld requested a review from a team as a code owner October 9, 2026 14:05
Comment thread ldclient/impl/util.py
Seconds on the monotonic clock. Use this for measuring intervals and
durations; use the wall clock for functionality tied to time outside the process.
"""
return time.monotonic()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we use time.monotonic_ns() instead? (refer to https://docs.python.org/3/library/time.html#time.monotonic)

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.

2 participants