Skip to content

fix(serve): sample play_start_us from CLOCK_MONOTONIC_RAW - #282

Merged
OnFreund merged 1 commit into
Sendspin:mainfrom
kiwipaulrob:fix/serve-raw-clock
Oct 4, 2026
Merged

OnFreund merged 1 commit into
Sendspin:mainfrom
kiwipaulrob:fix/serve-raw-clock

Conversation

@kiwipaulrob

Copy link
Copy Markdown
Contributor

Problem

In sendspin/serve/coordinator.py, play_start_us is sampled with time.monotonic() -- but the aiosendspin server clock is CLOCK_MONOTONIC_RAW on Linux (aiosendspin/clock.py: not slewed by NTP/adjtime). play_start_us is an absolute timestamp consumed verbatim by workers/clients, so sampling it from a different clock domain introduces a fixed offset equal to every NTP adjustment since boot.

Evidence

On my Linux host the two clocks are currently 102.2 s apart, so the multi-worker serve path timestamps audio ~102 s off. One honest caveat: aiosendspin/clock.py notes the rate mismatch is capped at ~500 ppm and below backpressure tolerances -- but that covers relative durations, not an absolute epoch offset, which is what play_start_us is.

Fix

Adds a small server_now_us() helper (raw clock on Linux, monotonic_ns fallback elsewhere) and uses it for the two absolute-timestamp samples. Interval-only measurements (stats/health timers) deliberately stay on time.monotonic().

Verification

  • pytest tests/serve/test_coordinator.py: 7 passed; ruff check clean
  • Live check: helper returns the raw-domain value

Happy to adjust the approach if you prefer reusing an aiosendspin clock helper.

@kiwipaulrob
kiwipaulrob marked this pull request as ready for review September 30, 2026 22:30
@kiwipaulrob

Copy link
Copy Markdown
Contributor Author

Hi @OnFreund — these four are now ready for review (were drafts while I finished testing). Small, separate fixes:

One ask I can't do myself: the CI runs on all four are frozen at action_required (first-time contributor gate), so there are no checks to show yet. A one-time workflow approval would unblock them. Happy to adjust anything on review.

@OnFreund
OnFreund merged commit 16d522b into Sendspin:main Oct 4, 2026
2 checks passed
@OnFreund

OnFreund commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Thanks @kiwipaulrob. I approved and merged this one. The others either fail CI or have a review comment.

@maximmaxim345 maximmaxim345 added the bugfix Fixes a bug label Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants