Skip to content

device/telemetry: keep probing when the epoch fetch fails - #4143

Open
elitegreg wants to merge 5 commits into
mainfrom
gm/telemetry-pinger-epoch-fallback
Open

device/telemetry: keep probing when the epoch fetch fails#4143
elitegreg wants to merge 5 commits into
mainfrom
gm/telemetry-pinger-epoch-fallback

Conversation

@elitegreg

@elitegreg elitegreg commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Pinger.Tick fetched the current epoch first and returned early on failure, so one unreachable ledger RPC endpoint stopped all TWAMP probing. Probing is pure UDP and needs no ledger access — the epoch only builds the sample buffer's PartitionKey. During the 2026-07-29 outage (#4130) this cost up to 19 hours of latency samples per device across 23 mainnet-beta devices, for probes that would have succeeded the whole time.
  • The pinger now caches the last known epoch and falls back to it, so an RPC outage costs epoch precision rather than measurements. Samples buffer through the outage and flush once submission recovers.
  • The epoch is refreshed on its own loop instead of inline per tick. This matters as much as the fallback: a failing fetch burns ~130s across its retries and time.Ticker buffers only one tick, so an inline fetch swallowed roughly a dozen probe opportunities per failure. A cached fallback alone would have resumed probing at ~1/13th the sample rate. The refresh cadence defaults to -probe-interval, so the epoch RPC rate is unchanged.
  • Probing is refused only when no epoch has ever been fetched, or when the cached one is older than -max-epoch-staleness (default 12h). The bound is load-bearing: contrary to the issue text, stale-epoch samples across a rollover are not discarded — Submitter.Tick removes a past-epoch partition only when it is empty, and the program never validates the epoch against the clock (it is PDA seed material only). They land in the previous epoch's account, where the lake's derived event_ts places them past the real epoch boundary.
  • Both refusal cases log once rather than per tick, and repeated fetch failures collapse into fresh→stale and stale→fresh transitions. The per-attempt retry warning in getCurrentEpoch drops to Debug — at a 10s cadence it emitted thousands of lines across the outage, which is what [TRACKER] Telemetry outage 2026-07-29 — ledger RPC wedge and agent resilience #4130's "single-digit log lines per component" exit criterion is about.
  • New pinger_epoch_unavailable error type and a doublezero_device_telemetry_agent_epoch_cache_stale_age_seconds gauge, matching the shape of the existing CachingFetcher staleness metric.
  • Fixes device/telemetry: pinger aborts all probing when the epoch fetch fails #4125

Testing Verification

  • Five new subtests, all under -race: probing continues under the cached epoch while the fetch fails; the fallback and recovery each log exactly once; probing stops past MaxEpochStaleness with an injected clock; nothing is recorded and one error is logged when no epoch was ever fetched; and the probe loop holds its cadence while the epoch fetch hangs forever (the coalescing regression).
  • Verified the tests discriminate by mutating epochForTick back to the pre-fix inline-fetch-and-give-up behavior: four of the five failed. The recovery test initially passed under the mutation because pre-outage samples satisfied it, so it now asserts the stale partition keeps growing while the fetch is failing.
  • The two pre-existing epoch subtests pass unchanged — the retry-count and give-up-with-no-epoch behaviors are preserved by the inline first fetch.
  • Ran the pinger suite three times to check for timing flakiness in the Run-driven tests.
  • internal/netns/TestRunInNamespace_EmptyNameErrors fails identically on a clean main (needs privileges for setns); unrelated to this change.

The pinger fetched the current epoch before every tick and returned early
on failure, so one unreachable ledger RPC endpoint stopped all TWAMP
probing. Probing is pure UDP; the epoch only builds the sample buffer's
partition key. During the 2026-07-29 outage this cost up to 19 hours of
latency samples per device across 23 mainnet-beta devices.

Cache the last known epoch and refresh it on its own loop, so the probe
path never blocks on RPC. A failing fetch burns ~130s across its retries
and the probe ticker only buffers one tick, so an inline fetch also
swallowed roughly a dozen probe opportunities per failure.

Probing is refused only when no epoch has ever been fetched, or when the
cached one is older than -max-epoch-staleness (default 24h), past which a
rollover is likely enough that samples would be misattributed to the
previous epoch's account. Both cases log once rather than per tick, and
repeated fetch failures collapse into fresh/stale transitions.

Fixes #4125
@elitegreg
elitegreg marked this pull request as ready for review August 3, 2026 14:41
@elitegreg
elitegreg enabled auto-merge (squash) August 3, 2026 14:41
@elitegreg
elitegreg requested a review from Copilot August 3, 2026 16:18

Copilot AI left a comment

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.

Pull request overview

Improves resilience of the device telemetry agent’s TWAMP probe loop by decoupling epoch fetching from the probe tick path and falling back to a cached epoch during ledger RPC outages, preventing probe gaps like the 2026-07-29 incident.

Changes:

  • Cache the last known epoch in Pinger and keep probing during epoch fetch failures, with a configurable staleness bound and log-once transitions.
  • Add a background epoch refresh loop plus new metrics/error typing to make epoch-cache health observable.
  • Add targeted subtests covering fallback, recovery, refusal cases, staleness cutoff, and non-blocking behavior under a hung epoch fetch.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
controlplane/telemetry/internal/telemetry/pinger.go Adds epoch cache + refresh loop; probes use cached epoch and refuse only when unavailable/too stale.
controlplane/telemetry/internal/telemetry/pinger_test.go Adds subtests validating fallback, recovery, staleness refusal, log coalescing, and non-blocking probe cadence.
controlplane/telemetry/internal/telemetry/config.go Introduces MaxEpochStaleness config with defaulting in validation.
controlplane/telemetry/internal/telemetry/collector.go Plumbs MaxEpochStaleness and NowFunc into PingerConfig.
controlplane/telemetry/internal/metrics/metrics.go Adds epoch-cache staleness gauge and pinger_epoch_unavailable error type.
controlplane/telemetry/cmd/telemetry/main.go Adds -max-epoch-staleness flag wired into telemetry config.
CHANGELOG.md Documents the behavioral change and new flag in Unreleased notes.

Comment thread CHANGELOG.md Outdated
Resolves: #4128

Independent of #4144 and #4145 (different file), so this one branches
from `main`.

## Summary of Changes

- `ledgerPeerDiscovery.refresh` no longer empties the peer cache before
doing work that can fail. It cleared `p.peers` under the lock and then
called `LocalNet.Interfaces()`, so a transient failure there returned
with zero peers and `Pinger.Tick` iterated an empty slice, probing
nothing until a later refresh succeeded.
- The cache is now replaced only once the new list is built, and the
lock covers just that assignment rather than the whole build. The clear
was redundant with the existing assignment at the end of the happy path.
- Success path is unchanged.

## Diff Breakdown

| Category   | Files | Lines (+/-) | Net  |
|------------|-------|-------------|------|
| Tests      |     1 | +79 / -0    |  +79 |
| Core logic |     1 | +6 / -5     |   +1 |
| Docs       |     1 | +3 / -0     |   +3 |
| **Total**  |     3 | +88 / -5    |  +83 |

A one-line behavioral fix plus the regression test that pins it.

<details>
<summary>Key files (click to expand)</summary>

-
[`controlplane/telemetry/internal/telemetry/peers.go`](https://github.com/malbeclabs/doublezero/pull/4146/files#diff-9c369dff3cb79259b8bc34d8d952b923103baeef1515402614c23a997a06c286)
— drops the `p.peers = make(...)` clear, moves the mutex to wrap only
`p.peers = peers`, and leaves a comment that nothing in the build may
clear the cache

</details>

## Testing Verification

- New test lets the first refresh discover a peer, then fails every
subsequent `LocalNet.Interfaces()` call, and asserts `GetPeers()` still
returns the fully populated peer (link, device, tunnel, TWAMP port)
after at least three failed refreshes. It fails against the pre-fix
code, which returns an empty list.
- Existing peer discovery tests pass unchanged, covering the success
path and the skip cases.
- Package passes under `-race`, since the change moves what the mutex
covers.
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.

device/telemetry: pinger aborts all probing when the epoch fetch fails

2 participants