feat(exporter): remove standard mode, add optional duty-trace retention - #2895
Conversation
The exporter's "standard" mode is removed; exporters now always run full duty tracing (the former "archive" behavior). Standard mode was the default (EXPORTER_MODE=standard) yet a strict, and historically broken, subset: it served only the participant-store-backed /v1/exporter/decideds endpoint and was the source of the gossip-message blackhole (see #2886). All deployed exporters already run archive, so this removes a footgun without losing capability — archive is a superset (it serves the same decideds endpoint plus the trace endpoints). Changes: - exporter.Options: drop Mode/ModeStandard/ModeArchive; replace the standard-only RetainSlots with RetainEpochs (EXPORTER_RETAIN_EPOCHS, default 0 = retain indefinitely, matching today's archive behavior). - cli/operator: collapse the node-mode enum to operator/exporter; resolveMode no longer fails (no mode string to validate); always wire the duty-trace collector and read API for exporters; drop the standard-only participant-store pruning. - duties.Scheduler: drop the ArchiveMode option; the Attester handler is now registered for every exporter (full tracing), as before for operators. - validator.Controller: route non-committee messages to the trace collector whenever one is wired (exporter), else the legacy participant path (operator). - dutytracer: add DutyTraceStore.PruneSlot and a retention loop in Collector.Start that prunes trace data older than RetainEpochs (capped per tick; seeds forward on first run, so enabling retention does not retroactively sweep old history). Config back-compat: a stale EXPORTER_MODE / RetainSlots in existing configs is ignored (unknown fields), so nodes keep booting; the default flips from the (broken) standard to full tracing. Tests: handler-registration, mode resolution, exporter wiring, store PruneSlot, and collector retention-cursor logic updated/added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR removes the exporter's
Confidence Score: 4/5Safe to merge; the retention feature is best-effort by design and the two open gaps in the pruning cursor (acknowledged in prior review threads) are documented and don't affect correctness of the tracing path itself. The two previously flagged gaps in the retention cursor — in-memory state lost across restarts leaving expired slots permanently on disk, and the cursor silently advancing past slots whose PruneSlot call returned an error — remain unresolved. The PR explicitly documents them as best-effort behavior, but operators enabling EXPORTER_RETAIN_EPOCHS may see unbounded disk growth despite the setting. All other changes are clean and well-tested. operator/dutytracer/collector.go — retentionState cursor behavior on restart and on PruneSlot failure; the two open concerns from the prior review threads live here. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Process start] --> B{ExporterOptions.Enabled?}
B -- No --> C[modeOperator\nno collector]
B -- Yes --> D[modeExporter\nfull duty tracing]
D --> E[Compute retainSlots\nRetainEpochs × SlotsPerEpoch]
E --> F[Collector.Start\ncollector + retentionState]
F --> G{retainSlots == 0?}
G -- Yes --> H[Retain indefinitely\nno pruning]
G -- No --> I[Per-tick: advance retentionState]
I --> J[Update atomic floor\nreject expired writes]
I --> K[Prune cursor up to maxPrunePerTick\nslots per tick]
C --> L[handleNonCommitteeMessages\nlegacy participant store]
D --> M[traceCollector.Collect\nfull trace path]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Process start] --> B{ExporterOptions.Enabled?}
B -- No --> C[modeOperator\nno collector]
B -- Yes --> D[modeExporter\nfull duty tracing]
D --> E[Compute retainSlots\nRetainEpochs × SlotsPerEpoch]
E --> F[Collector.Start\ncollector + retentionState]
F --> G{retainSlots == 0?}
G -- Yes --> H[Retain indefinitely\nno pruning]
G -- No --> I[Per-tick: advance retentionState]
I --> J[Update atomic floor\nreject expired writes]
I --> K[Prune cursor up to maxPrunePerTick\nslots per tick]
C --> L[handleNonCommitteeMessages\nlegacy participant store]
D --> M[traceCollector.Collect\nfull trace path]
Reviews (2): Last reviewed commit: "test(exporter): fix lint + raise patch c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // lastPrunedSlot tracks retention progress; seeded lazily on the first eligible tick so we | ||
| // enforce retention going forward without an expensive backfill of pre-existing history. | ||
| var lastPrunedSlot phase0.Slot | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-ticker.Next(): | ||
| currentSlot := ticker.Slot() | ||
| c.evict(currentSlot) | ||
| if retainSlots > 0 { | ||
| lastPrunedSlot = c.pruneExpired(currentSlot, phase0.Slot(retainSlots), lastPrunedSlot) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
lastPrunedSlot is a local variable initialized to zero on every process start. After a restart, pruneExpired treats the zero value as "first run" and seeds the cursor to the current boundary (currentSlot - retainSlots), skipping all slots that fell outside the window since the last run's pruning stopped. Those slots are never visited again because the forward-only cursor never goes back.
Concretely: if the node was pruning up to slot 1 000 before a restart and comes back at slot 1 200 with retainSlots=160 (10 epochs × 32), slots 1 000–1 039 are permanently skipped. Each rolling deploy repeats this, so storage can grow without bound regardless of the configured RetainEpochs. Persisting the cursor in the KV store (or seeding from the last-known pruned slot on startup) would close this gap.
| for s := lastPruned; s < boundary && pruned < maxPerTick; s++ { | ||
| if err := c.store.PruneSlot(s); err != nil { | ||
| c.logger.Warn("failed to prune expired duty traces", fields.Slot(s), zap.Error(err)) | ||
| } | ||
| pruned++ | ||
| lastPruned = s + 1 | ||
| } |
There was a problem hiding this comment.
lastPruned is incremented to s + 1 even when c.store.PruneSlot(s) returns an error. A slot whose data cannot be removed (e.g., due to a transient or persistent DB write error) is silently skipped — its data stays on disk indefinitely while the cursor moves on. The warning log is the only signal, and there's no retry path. Under sustained DB pressure, slots pile up past the retention boundary with no recovery mechanism.
Live verification — stage mainnet exporter (node 1002)Deployed
Retention ( |
Move the duty-trace retention logic out of an ad-hoc cursor threaded through a method into a small, self-contained tracePruner, and expand its test to cover every safeguard. Behavior is unchanged; this keeps the retention path production-grade (instrumented + edge-case-guarded) rather than trimming the safety logic for brevity: - underflow guard (currentSlot <= retainSlots) — no unsigned slot wraparound early in the chain's life; - seed-forward on first activation — enabling retention on a live chain does not backfill millions of historical (mostly empty) slots; - per-tick cap (maxPrunePerTick) — a large boundary jump after a pause cannot stall the eviction loop; - cursor catch-up — a coalesced/late tick reclaims skipped slots instead of leaking them; - debug log on prune + duration metric via DutyTraceStoreMetrics. TestTracePruner exercises disabled, underflow, seed-forward, steady-state, and capped-catch-up cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update — retention refactor + steady-state verification (commit
|
| Check | Result |
|---|---|
Boots on 69cd7d8 |
✅ |
| Full-tracing config | ✅ exporter enabled (full duty tracing) retain_epochs=0 retain_slots=0 |
| Handler set | ✅ PROPOSER + SYNC_COMMITTEE + VOLUNTARY_EXIT + ATTESTER |
| Steady-state RSS (52 min uptime) | ✅ 5.80 GB — in line with the archive exporter it replaces (prod 6.44 GB, stage 6.0–6.3 GB) |
| Peers | ✅ 510 |
| CL/event sync | ✅ 100% |
Warm-up dropping buffered noise |
✅ cleared (0 in last 5m) |
| Non-benign ERROR logs (last 5m) | ✅ 0 |
Steady-state resource use confirms the expectation: removing standard mode and making the exporter a full tracer costs the same as the archive mode already deployed fleet-wide — the workload is identical minus the deleted dead code.
…n + polish
Independent review (Codex) of the retention work surfaced a real correctness gap
and several improvements:
- Recreation race (should-fix): after PruneSlot deleted a slot, a late message
took the disk-miss path in getOrCreate{Validator,Committee}Trace and re-saved
an empty trace, resurrecting the pruned slot. Introduce a single retained-slot
floor (retentionState, owned by Collector) that the collection and schedule
paths consult; messages/jobs for slots below the floor are dropped via a new
errExpiredSlot sentinel (Collect drops it silently). Reachable when
RetainEpochs is small enough that the floor falls inside the message-validation
window.
- Refactor: folded the ad-hoc tracePruner cursor into retentionState — one source
of truth for the floor (drop guard) and the disk-prune cursor.
- Honest semantics (should-fix): EXPORTER_RETAIN_EPOCHS is now documented as
best-effort, forward-only retention (history aging out during downtime is not
reclaimed after restart). Persisted-cursor GC left as a follow-up.
- Deprecation visibility (should-fix): warn at startup when removed
EXPORTER_MODE / EXPORTER_RETAIN_SLOTS env vars are set, so a stale value isn't
silently ignored.
- Simplification: PruneSlot drops scheduled data with a single sd+slot prefix
(makeScheduledSlotPrefix) instead of iterating roles.
- Tests: TestRetentionState now covers the floor/expired guard; new
TestCollector_dropsExpiredSlot proves the hot path refuses expired slots;
TestPruneSlot covers multiple validator roles. Stale comments fixed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Independent Codex review + fixes (commit
|
- lint: use the promoted networkConfig.SlotsPerEpoch instead of the embedded networkConfig.Beacon.SlotsPerEpoch selector (staticcheck QF1008). - extract runScheduleWorker's body into processScheduleJob so the retention guard is unit-testable. - add tests covering the previously-uncovered new code: retention floor advance via Start, schedule-job skip for expired slots, the metrics-wrapper PruneSlot, PruneSlot DB-error aggregation, the deprecation warning, and an end-to-end Collect drop of an expired-slot message. Patch coverage 70.8% -> 92.0%; golangci-lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptile pls re-review |
| return nil | ||
| } | ||
| if errors.Is(err, errExpiredSlot) { | ||
| return nil // message is for a slot outside the retention window; drop silently |
There was a problem hiding this comment.
Might be worth emitting a Debug log or a dropped_expired counter here rather than dropping silently.
As-is, a message dropped because its slot aged out is indistinguishable from a too-tight EXPORTER_RETAIN_EPOCHS quietly discarding live traffic — there's no signal to tell correct retention from a misconfiguration eating real data.
The same goes for the errExpiredSlot path in collectLateMessage.
Non-blocking btw.
There was a problem hiding this comment.
Good call on the observability — a dropped_expired counter (plus a Debug line in the late path, where the err = nil currently suppresses the deferred error log) is cheap.
One thing on the framing though: I don't think this can actually mask a too-tight window eating live traffic. Retention is opt-in and off by default (EXPORTER_RETAIN_EPOCHS=0 → floor stays 0, so expired() is always false and nothing is ever dropped), and when it's enabled the expired-drop only fires for messages older than the entire window. Live traces arrive within a few slots; to drop anything recent you'd have to set the window to ~1 epoch, which is an obviously-broken value rather than a subtle misconfig.
So I'd add it for debuggability of the retention feature itself — catching a floor-computation regression, or just confirming pruning behaves — rather than as a guard against silently discarding real data, since that's already prevented by the off-by-default + whole-window design.
| mode nodeMode | ||
| } | ||
|
|
||
| // isExporter reports whether the node runs as an exporter (standard or archive) rather than as an |
There was a problem hiding this comment.
| // isExporter reports whether the node runs as an exporter (always in former archive mode) rather than as an |
There was a problem hiding this comment.
Small nitpick for consistency, very optional.
|
Stage-hoodi deploy test — no regression detected ✅ Deployed PR branch ( VOLUNTARY_EXIT (live exit event, validator 838944)
Message validation drops Duty scheduler Peer connectivity: 86-88 peers per node (consistent with fleet exporter baseline of ~87-94). |
651052b
into
fix/exporter-duty-store-blackhole
4 participants
Summary
Follow-up to #2892. Removes the exporter's standard mode — exporters now always run full duty tracing (the former archive behavior) — and adds an optional, default-off retention window for the on-disk duty-trace store.
Why
EXPORTER_MODEdefaulted tostandard, yet standard was a strict — and historically broken — subset of archive:/v1/exporter/decidedsendpoint (no trace endpoints).Every deployed exporter (prod mainnet, stage mainnet, all hoodi exporters) already runs
archive. Verified live: the production mainnet exporter handles all duty classes archive does; archive is a functional superset of standard (samedecidedsendpoint + the trace endpoints). So removing standard deletes a real footgun (a default that silently blackholes) without losing any capability.What changed
exporter.Options: dropMode/ModeStandard/ModeArchive. Replace the standard-onlyRetainSlotswithRetainEpochs(EXPORTER_RETAIN_EPOCHS, default0= retain indefinitely — matches today's archive behavior).cli/operator: node-mode enum collapses tooperator/exporter;resolveModecan no longer fail; the duty-trace collector + read API are always wired for exporters; the standard-only participant-store pruning is removed.duties.Scheduler: drop theArchiveModeoption — the Attester handler is registered for every exporter (full tracing), unchanged for operators.validator.Controller: route non-committee messages to the trace collector whenever one is wired (exporter), else the legacy participant path (operator) — nil-safe viatraceCollector != nil.dutytracer: newDutyTraceStore.PruneSlot(slot)(single prefix-drop per key group) and a retention loop inCollector.Startthat prunes trace data older thanRetainEpochs. Pruning is capped per tick and seeds forward on first run — enabling retention does not retroactively sweep pre-existing history.Compatibility
EXPORTER_MODE/RetainSlotsin existing configs is simply ignored (unknown fields), so nodes keep booting.standardto full tracing — which is what the fleet already runs./v1/exporter/decidedshandler are untouched; only the exporter stops using standard mode.Tests
operator/duties), mode resolution + exporter wiring (cli/operator).TestPruneSlot(store removes all trace kinds for a slot, leaves adjacent slots intact) andTestCollector_pruneExpired(retention cursor: seed-forward, per-tick advance, per-tick cap).go build ./...,go vet, and affected unit suites pass.Verification status
Unit-tested and built clean. Live deploy verification on a stage exporter is pending (vnet tunnel was down at PR-creation time) — will confirm boot (full tracing), trace endpoints, health, and that the now-ignored
EXPORTER_MODEin the deploy template still boots cleanly.🤖 Generated with Claude Code