feat(replication): structured fire telemetry for recovery nets (W1 T1, #431)#525
feat(replication): structured fire telemetry for recovery nets (W1 T1, #431)#525kriszyp wants to merge 1 commit into
Conversation
…#431) First stage of the watchdog-demotion plan (#431 design comment): every recovery mechanism now logs, at fire time, what the authoritative shared-memory truth said and which earlier layers had already engaged — the evidence the demotion soak needs to tell "sole detector" fires from redundant ones. Telemetry only; no recovery decision consults the new state. - formatTruthSnapshot (exported, pure): one-line truth snapshot (connected, state, liveness age, last close code) appended to the three worker-local watchdog fire logs (byte-silence receive watchdog, pause-stall watchdog, copy-progress watchdog). - describePriorSignals (exported, pure): the last recovery mechanism and/or truth correction applied to an entry, with ages; 'prior=none' marks a sole-detector fire. - The main-thread nets (wedge reconcile, receive-stall net) stamp entry.lastRecovery and log per-entry truth snapshot + prior signals; the truth-reconcile corrections stamp entry.lastTruthCorrection so a subsequent wedge fire shows it was truth-driven. Verified before scoping: retrieval connections use the GET_RECORD/RESPONSE request path, never the audit-apply loop, so the receive-progress slots are not contaminated by non-owners and need no gating here (the #523 re-review follow-up is resolved as not-applicable). Cross-thread prior-signal visibility (worker fires stamping shared memory for the main thread to read) deliberately deferred to T2 — it would burn reserved slots 13-15 and the soak may not need it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces telemetry logging for replication connection watchdogs and reconciliation networks to assist in debugging connection failures. It adds formatting functions for connection truth snapshots and prior recovery signals, integrates them into warning logs, and includes unit tests. The review feedback identifies a critical issue where an unhandled exception in the telemetry-gathering helper could block the connection recovery path, suggesting a try-catch block as a safeguard. Additionally, it recommends adding defensive checks in describePriorSignals to safely handle null or undefined entries and prevent NaN values in logs.
| const truthSnapshotForLog = () => { | ||
| const status = getSharedStatus(); | ||
| return formatTruthSnapshot(status && deriveConnectionTruth(status)); | ||
| }; |
There was a problem hiding this comment.
While formatTruthSnapshot is a pure formatter designed not to throw, truthSnapshotForLog invokes getSharedStatus() and deriveConnectionTruth(). If getSharedStatus() or any underlying call throws (e.g., if auditStore is in an unexpected state or getUserSharedBuffer is unavailable), this will throw an unhandled exception directly inside the watchdog's onSilence / onStall recovery callbacks. This would prevent critical recovery actions like forceReconnect() or ws.terminate() from executing, leaving the connection permanently wedged. Wrapping the telemetry generation in a try-catch block ensures that telemetry failures never disrupt the recovery path.
const truthSnapshotForLog = () => {
try {
const status = getSharedStatus();
return formatTruthSnapshot(status && deriveConnectionTruth(status));
} catch (error) {
return 'truth=error';
}
};| export function describePriorSignals( | ||
| entry: { lastRecovery?: { mechanism: string; at: number }; lastTruthCorrection?: { direction: string; at: number } }, | ||
| now: number | ||
| ): string { | ||
| const parts: string[] = []; | ||
| if (entry.lastRecovery) | ||
| parts.push(`${entry.lastRecovery.mechanism} ${Math.round((now - entry.lastRecovery.at) / 1000)}s ago`); | ||
| if (entry.lastTruthCorrection) | ||
| parts.push( | ||
| `truth-corrected-${entry.lastTruthCorrection.direction} ${Math.round((now - entry.lastTruthCorrection.at) / 1000)}s ago` | ||
| ); | ||
| return parts.length ? `prior={${parts.join('; ')}}` : 'prior=none'; | ||
| } |
There was a problem hiding this comment.
To enforce defensive programming and prevent potential runtime TypeErrors, describePriorSignals should safely handle null or undefined inputs for the entry parameter. Additionally, checking that at is a valid number before performing arithmetic operations prevents NaN values from propagating into the telemetry logs.
| export function describePriorSignals( | |
| entry: { lastRecovery?: { mechanism: string; at: number }; lastTruthCorrection?: { direction: string; at: number } }, | |
| now: number | |
| ): string { | |
| const parts: string[] = []; | |
| if (entry.lastRecovery) | |
| parts.push(`${entry.lastRecovery.mechanism} ${Math.round((now - entry.lastRecovery.at) / 1000)}s ago`); | |
| if (entry.lastTruthCorrection) | |
| parts.push( | |
| `truth-corrected-${entry.lastTruthCorrection.direction} ${Math.round((now - entry.lastTruthCorrection.at) / 1000)}s ago` | |
| ); | |
| return parts.length ? `prior={${parts.join('; ')}}` : 'prior=none'; | |
| } | |
| export function describePriorSignals( | |
| entry: { lastRecovery?: { mechanism: string; at: number }; lastTruthCorrection?: { direction: string; at: number } } | null | undefined, | |
| now: number | |
| ): string { | |
| if (!entry) return 'prior=none'; | |
| const parts: string[] = []; | |
| if (entry.lastRecovery && typeof entry.lastRecovery.at === 'number') { | |
| parts.push(`${entry.lastRecovery.mechanism} ${Math.round((now - entry.lastRecovery.at) / 1000)}s ago`); | |
| } | |
| if (entry.lastTruthCorrection && typeof entry.lastTruthCorrection.at === 'number') { | |
| parts.push( | |
| `truth-corrected-${entry.lastTruthCorrection.direction} ${Math.round((now - entry.lastTruthCorrection.at) / 1000)}s ago` | |
| ); | |
| } | |
| return parts.length ? `prior={${parts.join('; ')}}` : 'prior=none'; | |
| } |
|
Reviewed; no blockers found. |
|
Cross-model review (standard — Gemini Adjudicated false positives (diff-only artifacts): One valid low-priority suggestion deliberately not taken: DRYing the near-identical telemetry blocks in the wedge and stall nets into a shared helper — the two blocks differ slightly (clock + node-name source) and the duplication is five lines; folding it would add churn to a reviewed diff for cosmetic gain. Fair game for the T2 PR, which touches the same region. Note for reviewers: stacked on #523 — base is |
What
Stage T1 of the watchdog-demotion plan (design on #431). Every recovery mechanism now logs, at fire time, what the authoritative shared-memory truth said and which earlier layers had already engaged — the evidence the demotion soak needs to distinguish sole-detector fires from redundant ones. Telemetry only; no recovery decision consults the new state.
Stacked on #523 (uses its truth-reconcile corrections and ownership-gated slots); base is
kris/w1-truth-shadow.formatTruthSnapshot(exported, pure) — one-line truth snapshot (connected,state, liveness age, last close code) appended to the three worker-local watchdog fire logs: byte-silence receive watchdog, pause-stall watchdog, copy-progress watchdog. A fire while truth readsconnected+ fresh liveness is an invariant-violation candidate; a fire while truth already reads down means the truth-driven path had (or should have) taken over.describePriorSignals(exported, pure) — the last recovery mechanism and/or truth correction on the entry, with ages;prior=nonemarks a sole-detector fire, making the T3 demotion criterion (zero sole-detector fires over the soak) a grep.entry.lastRecoveryand log per-entry truth snapshot + prior signals; the truth-reconcile corrections stampentry.lastTruthCorrection, so a wedge fire that followed a down-correction visibly reads as truth-driven.Verification that de-scoped the follow-up
The #523 re-review flagged a potential contamination of the ungated receive-progress slots by retrieval connections. Traced before scoping T1: retrieval connections use the
GET_RECORD/GET_RECORD_RESPONSErequest path (awaitingResponse), never the audit-apply loop — audit records only flow to a socket that sent aSUBSCRIPTION_REQUEST, which only an owning subscription does. Not applicable; no gating added.Cross-thread prior-signal visibility (worker fires stamping shared memory for main to read) is deliberately deferred to T2 — it would burn reserved slots 13–15 and the soak may not need it.
Tests
unitTests/replication/fireTelemetry.test.mjs— 8 cases pinning the grep-able log shapes (unavailable buffer,liveness: never, close-code inclusion,prior=none, single and combined prior signals). FullunitTests/replication/suite: 278 passing.Part of #430 · W1 #431 · stacked on #523.
🤖 Generated with Claude Code