Skip to content

Commit ed2714c

Browse files
committed
TODO: agent tunnel store-and-forward robustness follow-ups from the #978 review
1 parent 702de62 commit ed2714c

1 file changed

Lines changed: 34 additions & 0 deletions

File tree

TODO.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,40 @@ Considered and deliberately NOT implemented (rationale on record so it isn't re-
431431
pumps opaque bytes. Command safety lives server-side (parameterized command
432432
construction); the gateway-side option is `authorized_keys` forced commands.
433433

434+
### Agent Tunnel Store-and-Forward: Robustness Follow-ups (#978 steady-state review)
435+
436+
The ack-based result buffer shipped in #978 is sound in steady state; these are edge-case behaviors
437+
found in review, none blocking. (The review's top finding - agent clock skew silently disabling a
438+
site's alerting via the AlertFreshness gate - is fixed: `AgentProbeResultSink` now warns hourly per
439+
site when samples on a long-connected tunnel skip alert evaluation.)
440+
441+
- [ ] **Poison frame becomes a reconnect-replay loop.** A frame that deterministically throws in
442+
`RecordBatchAsync`/`RecordSnmpBatchAsync` (corrupt site DB, disk full, throwing Influx config)
443+
tears down the tunnel read loop, never gets acked, and replays from the front of the buffer on
444+
every reconnect - head-of-line blocking newer data until the 12 h age cap evicts it. Pre-#978 the
445+
frame was simply lost on teardown, which "recovered". Fix shape: per-frame try/catch that acks
446+
(or dead-letters with a log) after N failed attempts, so one bad frame can't wedge the site's
447+
pipeline. Transient errors (DB lock, Influx blip) should still retry, so N > 1.
448+
- [ ] **Ack precedes durable persistence.** The server acks after `RecordBatchAsync` returns, but
449+
Influx writes may still sit in the client's write buffer - a server crash in that window loses
450+
data the agent has already trimmed. Narrow window, crash-only. Options: ack after an Influx
451+
flush, or accept and document the window (probably fine for monitoring data).
452+
- [ ] **False-stale blip window.** `StaleThreshold` (45 s) over a 30 s heartbeat leaves 15 s of
453+
margin. A healthy agent silent >45 s (host suspend, long GC pause, forward server clock step)
454+
gets proxy opens refused and its console flipped to awaiting-agent; recovery is the next
455+
heartbeat plus up to 60 s for the config-refresh loop to reconnect the console. The open breaker
456+
already clears instantly on fresh inbound - consider un-flipping the console on fresh inbound
457+
too, instead of waiting for the 60 s refresh tick.
458+
- [ ] **Buffered non-result frames are never directly acked.** `SnmpOidResult` (OID test replies)
459+
ride the ResultBuffer, but the server only acks ProbeResults/SnmpResults frames - other frame
460+
types are trimmed only cumulatively by a later result ack. On a quiet site (no monitoring flow)
461+
one can linger and replay on every reconnect until the 12 h age cap. Harmless today (the server
462+
ignores stale request ids), but acking every sequenced frame uniformly removes the class.
463+
- **Relevant code:** `AgentTunnelService` read loop (ack sites, `WatchLivenessAsync`),
464+
`TunnelClient.DrainResultsAsync`, `ResultBuffer`, `AgentProbeResultSink` (`AlertFreshness`,
465+
`NoteAlertEvaluationSkipped`), `AgentTunnelProxyService` (open breaker),
466+
`UniFiConnectionService.NoteTunnelUnreachableAsync`.
467+
434468
### Credential Key: Hardening Follow-ups
435469

436470
Self-hosted project, so keep this proportional. The at-rest credential key is a random

0 commit comments

Comments
 (0)