Support fallback NTP servers for edge time sync (#79)#82
Open
ff225 wants to merge 1 commit into
Open
Conversation
A single remote NTP server (e.g. the public pool) can introduce synchronization errors that hurt telemetry accuracy. All three transports (http, websocket, mqtt) now try an optional ordered list of fallback servers -- e.g. a local NTP server first -- before giving up on sync, sharing the retry/backoff logic via a new server.communication.ntp_sync module. Closes #79
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.
Summary
A remote NTP server (the public pool configured in
settings.yaml) can carry more time-synchronization error than a server on the local network. This adds support for an ordered list of fallback NTP servers per transport, so a local server can be configured as primary with the existing remote pool kept as a fallback (or vice versa) -- reducing the chance that transient unreachability of one server degrades sync quality or forces a fall back tooffset=0.0.Changes
server.communication.ntp_syncmodule: sharedsync_with_ntp_fallback(startup, retries every server each round with exponential backoff) andresync_ntp_fallback(periodic re-sync, tries each server once) helpers, extracted from the three near-identical implementations inhttp_server.py,websocket_server.py, andmqtt_client.py.ntp_fallback_servers: list[str]constructor arg and try[ntp_server, *ntp_fallback_servers]in order instead of only the single configured server.src/sciot/config.py: new optionalcommunication.{http,websocket,mqtt}.ntp_fallback_serversconfig key (list of non-empty strings), validated via a new_optional_string_listhelper.src/server/edge/run_edge.py: wires the new config key through to each transport constructor.src/server/settings.yaml: documents the new key with a commented-out example under each transport'sntp_server.Backwards compatible: the new parameter defaults to
None/[]everywhere, so existing configs withoutntp_fallback_serversbehave exactly as before (single server, same retry/backoff schedule).Test plan
tests/unit/test_ntp_sync.py: primary-server success, fallback on primary failure, all-servers-unreachable (0.0 offset), and periodic resync variants -- all with a mockedntplib.NTPClientand mockedsleep, no real network.tests/unit/test_config_validation.py: new cases for invalidntp_fallback_servers(wrong type / empty string) on each transport, and a valid-list acceptance test.uv run pytest tests/unit tests/integration/test_client_resilience.py tests/integration/test_client_lifecycle.py tests/integration/test_variance_and_local_inference.py tests/test_mqtt_client tests/test_offloading_algo-- 223 passed, 1 skipped.ruff checkon all changed files -- clean.Closes #79