Skip to content

alerting: node-health detectors, warnings mirroring, and gauges - #481

Merged
bkeroack merged 10 commits into
masterfrom
feature/a3-health-detectors
Aug 2, 2026
Merged

alerting: node-health detectors, warnings mirroring, and gauges#481
bkeroack merged 10 commits into
masterfrom
feature/a3-health-detectors

Conversation

@bkeroack

Copy link
Copy Markdown
Contributor

PR 2 of the A3 alerting stack (SATD_ALERTING_DESIGN.md §3). Stacked on #480 — merge that first.

PR 1 added the StatusEvent wire schema with no emitters. This adds the detectors that fill it.

What

satd now watches six conditions about itself and reports each through three surfaces simultaneously, driven from one state machine so they cannot disagree:

  • a status streaming event (category bit 16),
  • an entry in getwarnings — which fires the Core-compatible alertnotify hook,
  • a satd_alert_active{kind="..."} gauge on /metrics.
kind severity raises clears
ibd_complete info IBD finishes one-shot
tip_stall critical no block for alerttipstallseconds, outside IBD next block connects
disk_low critical free space under alertdiskfreemb 1.5× the floor
mempool_congested warning alertmempoolfullpct of the byte cap below 75 % of the raise line
peer_floor warning under alertpeerfloor for 60 s at/above for 60 s
deep_reorg critical reorg ≥ alertreorgdepth blocks one-shot

Decisions worth reviewing

  • Level-triggered with a real hysteresis gap. Clearing at the same value that raises turns a metric hovering at the threshold into a pager storm, so every standing condition has a ratio, a hold time, or both between its two lines. Fixed constants — six raise thresholds is already plenty of operator surface.
  • Durability by re-evaluation. Status events aren't replayable, so what makes health alerting at-least-once across a restart is that the task re-evaluates from scratch and re-raises anything still standing. A condition that raised and cleared while nobody was listening is stale by definition. Corollary, documented in the manual: a subscriber that attaches after a raise won't see it until the condition changes — getwarnings is the current-state query. (Snapshot-on-subscribe is deferred per design §8.)
  • API runtime, never the consensus core (design D7). A stall detector whose poll can be delayed by block connection is the one thing it must not be.
  • deep_reorg reports the true depth. ChainEvent::Reorg carries old/new tip heights but no fork point, so the detector counts the reorg's disconnects — exactly old_height − fork_height, the same number ReorgRecord stores — rather than reporting the threshold it crossed. A truncation reorg (invalidateblock) has no replacement chain to connect, so a count that goes quiet is finalized by the poll loop instead of waiting for a connect that may never come.
  • Thresholds are hot-reloadable, because an operator retuning an alert is usually doing it while it's firing. Disabling a threshold also clears a standing condition rather than stranding a warning nothing will ever retract.
  • ibd_complete deliberately creates no warning — it's good news, and a warning for it would sit in getwarnings forever with nothing to clear it.
  • tip_stall's clock is seeded at startup, not from the tip timestamp. A node that was down for hours has a stale tip but connects its backlog within seconds; seeding from the tip would page for a stall that is really a restart.
  • satd_disk_free_bytes is omitted, not zeroed, when the filesystem can't be interrogated — a zero there is exactly the alarm an operator must not be shown falsely. Same reason render_health_metrics emits nothing at all when no detector is running.

Also generalizes free_disk_bytes out of the three index runners that had each grown a private copy, and adds satd_tip_last_connect_age_seconds / satd_disk_free_bytes. Disk headroom previously had no metric or alerting — which is how a silent disk-fill wedged a dogfood node in May.

Config

alerttipstallseconds=3600, alertdiskfreemb=10240, alertmempoolfullpct=90, alertpeerfloor=3, alertreorgdepth=3. All hot-reloadable; 0 disables a detector. Registered in KNOWN_CONFIG_KEYS and the reload table, so the anti-drift coverage tests cover them.

Acceptance (design §10 PR 2)

  • E2E raise/clear over a real socket: tip_stall raise→clear, disk_low raised by a live SIGHUP threshold change (which also covers the reload path), deep_reorg's derived depth/fork height, and the explicit-only category guarantee (a categories=0 subscriber sees chain events but never a status event while one is firing).
  • getwarnings gains/loses alert.* in lockstep with the events.
  • Detector-off (=0) emits nothing and clears anything standing.
  • Threshold defaults come from one source (node::health::defaults), asserted from the config layer.
  • clippy --workspace --all-targets --all-features clean.

peer_floor is intentionally not covered E2E — it holds 60 s in either direction by design, which is longer than an E2E test should sit; its logic is unit-tested.

Local test run: green except the 10 pre-existing test_address_index_backfill_* regtest cases, which fail on this machine's disk-space guard (needs ~80 GB, 68 GB free) and are unrelated.

🤖 Generated with Claude Code

https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH

@bkeroack

Copy link
Copy Markdown
Contributor Author

Round-1 review fix (Medium — ops reliability): tip_stall now also clears from the poll path when the tip age falls back under the threshold, not only on BlockConnected.

The event-driven clear stays as the fast path — the alert should lift the instant the chain moves. The problem was the other direction: a node that genuinely is not receiving blocks has no event to clear on, and raising alerttipstallseconds via SIGHUP is the documented way to quiet a firing alert. Without a poll-side clear the condition, its getwarnings entry, the satd_alert_active{kind="tip_stall"} gauge and the stream state all stayed raised until some future block. Every other standing detector already cleared from the poll.

New e2e test raises the alert via SIGHUP, then raises the threshold via SIGHUP and asserts the clear arrives with no block mined. Verified it fails without the change. Manual and config-reference updated.

@bkeroack
bkeroack force-pushed the feature/a3-status-events branch from 15b229f to c20643a Compare July 25, 2026 15:46
@bkeroack
bkeroack force-pushed the feature/a3-health-detectors branch 2 times, most recently from 04cb6da to 3f67877 Compare July 30, 2026 19:10
@bkeroack

bkeroack commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Implemented the -connect fix discussed in review (commit 7f8c0e23).

peer_floor_for now takes the -connect= count and caps the default at it:

pub fn peer_floor_for(network: bitcoin::Network, connect_peers: usize) -> u64 {
    match network {
        bitcoin::Network::Regtest => 0,
        _ if connect_peers > 0 => PEER_FLOOR.min(connect_peers as u64),
        _ => PEER_FLOOR,
    }
}

Why. -connect= pins the node to exactly the addresses named and suppresses both DNS seeding and the fixed seeds (satd/src/main.rs:2918, :2946), so a node given one upstream can never reach a floor of 3 — no code path is left that would add a peer. The raise would stand forever in getwarnings, -alertnotify, the TUI modal, and getblockchaininfo.warnings, which wallet software renders to end users. satd_alert_active{kind="peer_floor"} pinned high also stops distinguishing peer-starved from configured-that-way.

This is the regtest trap in different clothes, and peer_floor_for's own doc comment already argued the case — it just keyed off the network when the condition is a property of the configuration.

Capped, not disabled, so the alert still does the one useful thing it can here: report a -connect= node that had all its configured peers and then lost one. An explicit alertpeerfloor overrides in either direction, including to 0.

The default had to move out of the Config struct literal — that literal has already moved connect by the time it reaches the alert_peer_floor field.

Tests (both run against the previous behavior and confirmed failing there):

  • node: peer_floor_default_is_disabled_only_on_regtest extended — 1 and 2 -connect peers give floors of 1 and 2, 8 caps at 3, regtest stays 0.
  • satd: new the_peer_floor_default_follows_the_connect_list — CLI and bitcoin.conf connect= paths, the cap, explicit-override in both directions, and regtest.

Docs updated in the same commit: CHANGELOG.md, docs/release-notes/0.5.0-pre.md, and the alertpeerfloor row in docs/manual/src/config-reference.md.

Stack rebased on top: #482#484#485#486, all force-pushed with --force-with-lease, bases unchanged.

@bkeroack

bkeroack commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Option A implemented for alerttipstallseconds (commit 5a978b9c) — finding 2(a).

pub fn tip_stall_for(network: bitcoin::Network) -> u64 {
    match network {
        bitcoin::Network::Regtest => 0,
        _ => TIP_STALL_SECS,
    }
}

This was the last threshold in the set without a network-conditional default; peer_floor_for and reorg_depth_for already turned regtest off for the same reason. last_connect is seeded at detector start and advanced only by BlockConnected, so a regtest node idling an hour between test runs raised a critical alert — pinning getwarnings, holding has_errors() true, and putting up the TUI's blocking modal on a chain where having no blocks is the resting state.

Test networks keep the hour rather than being relaxed the way reorg_depth_for relaxes them: unlike a shallow reorg, going an hour without a block is not an ordinary property of thin hashrate — signet and testnet4 are still expected to make blocks.

Finding 2(b) — the mainnet rate — is deliberately not changed. At a 600 s mean interval, P(gap > 3600 s) = e⁻⁶ ≈ 0.25 %, so a healthy node raises roughly every 2.8 days. The reviewer is right about the arithmetic and right that an error-severity page twice a week is the alert-fatigue mechanism this codebase warns about elsewhere. But 3600 is documented on TIP_STALL_SECS as an intentional trade, and raising it to 7200 (≈ once every 3 years) costs an hour of detection latency — that is a product call, not a review fix, so it stays the operator's to tune via alerttipstallseconds. Happy to revisit.

Tests: tip_stall_default_is_disabled_only_on_regtest in node, and the regtest/mainnet/signet assertions in health_alert_threshold_defaults_and_parsing. That test previously asserted 3600 on a regtest config and passed, so it is a direct negative check on the new value.

Docs: CHANGELOG.md, 0.5.0-pre.md (the "two defaults are network-dependent" paragraph is now three), and the alerttipstallseconds row in config-reference.md.

Stack rebased: #482#484#485#486, force-pushed with --force-with-lease, bases unchanged. One conflict in config-reference.md where #482 adds the alertfile row adjacent to this one — resolved keeping both.

bkeroack and others added 10 commits August 1, 2026 18:00
PR 2 of the A3 alerting stack (SATD_ALERTING_DESIGN.md §3). PR 1 added the
StatusEvent wire schema with no emitters; this adds the detectors.

satd now watches six conditions about itself — ibd_complete, tip_stall,
disk_low, mempool_congested, peer_floor, deep_reorg — and reports each
through three surfaces simultaneously: a `status` streaming event, an entry
in `getwarnings` (which fires the Core-compatible `alertnotify` hook), and a
`satd_alert_active{kind}` gauge. All three are driven from one state
machine, so they cannot disagree about node state.

Design points:

- Level-triggered, not edge-spammed. Every standing condition raises once
  on entry and clears once on recovery, with a fixed hysteresis gap between
  the two lines (a ratio, a hold time, or both) — clearing at the same value
  that raises turns a metric hovering at the threshold into a pager storm.
  ibd_complete and deep_reorg describe things that happened rather than
  states that persist, so they are one-shot edges with no clear.

- Durability by re-evaluation. Status events are not replayable, so what
  makes health alerting at-least-once across a restart is that the task
  re-evaluates from scratch and re-raises anything still standing.

- Runs on the API runtime, never the consensus core: a stall detector whose
  poll can be delayed by block connection is the one thing it must not be.

- Thresholds are hot-reloadable. An operator retuning an alert is usually
  doing it *because* it is firing; taking the node down to silence a pager
  is the wrong trade. Disabling a threshold also clears a standing
  condition rather than stranding a warning nothing will retract.

- deep_reorg reports the *true* depth. ChainEvent::Reorg carries the old and
  new tip heights but not the fork point, so the detector counts the reorg's
  disconnects — exactly `old_height - fork_height` — rather than reporting
  the threshold it crossed. A truncation reorg (invalidateblock) has no
  replacement chain to connect, so a count that goes quiet is finalized by
  the poll loop instead of waiting for a connect that may never come.

- Only conditions worth attention become warnings: `ibd_complete` is info
  and would otherwise sit in getwarnings forever with nothing to clear it.

Also generalizes `free_disk_bytes` out of the three index runners that had
each grown a private copy, and adds `satd_tip_last_connect_age_seconds` /
`satd_disk_free_bytes` — disk headroom previously had no metric or alerting
at all, which is how a silent disk-fill wedged a dogfood node in May.

Config: alerttipstallseconds=3600, alertdiskfreemb=10240,
alertmempoolfullpct=90, alertpeerfloor=3, alertreorgdepth=3; 0 disables.
E2E coverage over a real socket for tip_stall raise+clear, disk_low via a
live SIGHUP threshold change, deep_reorg's derived depth, and the
explicit-only category guarantee.
Round-3 review fixes folded in:

- `disk_low` and `mempool_congested` repeated the dead-band bug already fixed
  for `tip_stall`: both clear only on a hysteresis-widened predicate evaluated
  against the *current* threshold, so retuning a threshold to quiet a firing
  alert leaves the unchanged reading between the new raise and clear lines,
  where neither branch runs. For `mempool_congested` this was inescapable —
  `alertmempoolfullpct` clamps at 100 and the clear line is 0.75x the raise
  line, so past 75% occupancy no setting could clear it. A condition now
  records the threshold it was raised against and clears when that threshold
  moves such that it would no longer raise. The anti-flap gap still governs a
  reading that recovers on its own.

- The `tip_stall` IBD guard was unlatched. `is_initial_block_download()` is a
  function of wall-clock time against the tip header, so a node whose tip stops
  advancing crosses back into "IBD" 24h later and freezes the detector: every
  later poll returns before reaching the raise or clear branch, and a SIGHUP
  retune applies to the atomic and does nothing. Latch it — once caught up, a
  node is never in IBD again for this detector.

- `deep_reorg` abandoned its in-flight count on broadcast lag. The chain-event
  ring holds 64 entries and a reorg emits one event per disconnected and
  reconnected block, so lag becomes likely right around the depth where the
  alert starts to matter — making it least reliable for the largest reorgs.
  Fall back to the committed reorg-log record, which has the true fork height.

- `-alertnotify` fired only for the *first* `deep_reorg` of a process: the
  warning never clears, so the first-time-only dedup suppressed every later and
  possibly much deeper one, while the streaming and webhook surfaces reported
  all of them. Edge observations now notify on every occurrence
  (`record_recurring`); standing conditions keep the dedup.

- `check_disk` tested `statvfs` before the `floor == 0` disabled-clear, so an
  unmounted volume left a raised alert unclearable by any means including the
  documented escape hatch. `check_mempool` returned on `maxmempool=0` without
  clearing, stranding a raised alert the same way.

- `satd_alert_active` emitted a `# HELP`/`# TYPE` pair per series rather than
  per family. That is invalid text format, and strict parsers (`promtool check
  metrics`, `expfmt.TextParser`) reject the *entire page* on the duplicate,
  taking every unrelated satd metric with it. Split `metric` into a header and
  a sample writer, and add a test that asserts one header per family.

- `disk_low` carried the absolute watched path as a wire detail, so it reached
  every `status` subscriber and any push-notification body. It goes to the log
  instead.

- `alertpeerfloor` defaulted to 3 with no startup grace, so any node with fewer
  than 3 peers raised a permanent critical warning 60s in — including every
  regtest and single-node dev deployment. Default it to 0 on regtest and
  signet, and on the other networks hold off raising until 90s after startup or
  the first peer, whichever comes first.

Each behavioral fix has a test verified to fail without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
Round 4 of external review, scoped to the ~594 lines this branch added
during round 3 — code written as fixes and never itself reviewed.

`clear_if_threshold_relaxed` defeated the hysteresis it was written to
protect. The remembered threshold was stored only on the raise edge, so
retuning *down* while a condition stood left the slot stale, and the next
reading inside the hysteresis band read as "the operator moved the line":
raise at 93% against 90%, retune to 80%, ease to 78% — above the new clear
line of 60% — and the alert cleared, then re-raised on the next block. Now
refreshed on every evaluation where the raise predicate holds.

The Lagged reorg-log fallback rested on a false ordering claim. Its comment
said the log is written before any chain event is emitted; `perform_reorg`
does the reverse, and `ReorgLog::record` fsyncs before pushing to the ring
`history()` reads. So the record was typically absent at that instant, and
the newest one present belonged to an earlier reorg — reported with that
reorg's depth and fork point, after which state went Idle and the real reorg
was never reported. Now deferred a poll and matched on the abandoned tip
height from the marker; unmatched counts report as a lower bound rather than
vanishing, marked `depth_exact: false`.

The `floor == 0` return moved above the `statvfs` call, so
`satd_disk_free_bytes` was never populated with the alert disabled — losing
the gauge for operators who set `alertdiskfreemb=0` precisely because they
alert on it in Prometheus. Sample first, then decide.

The `threshold == 0` return sat above the `left_ibd` latch, so a node running
with `tip_stall` off never latched; enabling it on an already-wedged node
(tip >24h stale, hence "in IBD" again) wedged the detector permanently — the
exact failure the latch was added to prevent.

The peer startup grace ran concurrently with the hold rather than deferring
it, so it merely moved the raise to t=90s; and latching `saw_first_peer`
mid-grace fired the alert in the very poll the first peer arrived. The hold
now starts when the grace ends or the first peer lands.

`maxmempool=0` reported `reason=detector_disabled`, which means "the operator
turned this off" — a consumer suppressing that would swallow a zeroed mempool
cap. Now `mempool_cap_zero`.

Signet's peer floor goes back to 3. Signet is a public network with real
peers; a detector defaulted off is indistinguishable from a healthy one, as
`satd_alert_active{kind="peer_floor"}` reads 0 either way. Regtest keeps 0.

Tests: `check_disk` and `check_peers` and `check_tip_stall` are split into
value-taking halves, mirroring `check_mempool_values`, and the test-local
`sample_disk` reimplementation is gone — it mirrored the detector's branch
structure, so deleting the real `clear_if_threshold_relaxed` arm left every
disk test green. Six new tests, each verified to fail against the code it
guards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
`deep_reorg` reconstructed depth by counting `BlockDisconnected` events off
the chain broadcast, through a three-state machine with a lag-recovery
path. The design says otherwise — "depth from `ReorgRecord`" — and the
design is right: `perform_reorg` already writes the exact fork height,
depth, and reconnected chain, and fsyncs the record before pushing it to
the ring `history()` reads. Counting reconstructs by hand a number the node
recorded exactly, and gets it wrong in ways that all bite hardest at depth:

- The bus ring holds 64 entries and a reorg of depth D emits `2D + 2`
  events in one await-free burst, so a deep reorg truncates its own count —
  or drops the `Reorg` marker, after which the tracker sat in `Idle` and
  discarded the whole reorg silently.
- The lag fallback searched the log with `.find()`, and `history()` returns
  oldest-first. Two reorgs abandoning the same tip within the 300 s window
  — an ordinary tip race, or back-to-back `invalidateblock` — reported the
  earlier one's depth, which below threshold meant no event at all.
- `to_height` came from the first `BlockConnected` after the disconnect
  run. Reconnects are emitted oldest-first, so every reorg with a
  replacement chain reported `fork_height + 1` — *below* the old tip.
- A second reorg arriving mid-count overwrote the first, losing it.

Replaced with a watermark over the log: each poll reports records not seen
before, keyed on `(ts, old_tip)` because two reorgs can share a second. The
watermark is seeded from the clock at startup, since `deep_reorg` is an
edge event and D3 re-raises standing conditions across a restart, not
edges. `depth_exact` is gone — there is no inexact path left.

Separately, `tip_stall` no longer suppresses on IBD. The suppression was
latched against the predicate flapping, but the latch is per-process and a
restart is an operator's first move during a stall: a node restarted while
already wedged never observes a non-IBD tip, so the latch never armed and
the detector went silent permanently. `age_secs` already encodes the right
signal — a node that is really syncing connects blocks and stays under the
threshold on its own.

Seven new tests, each with its negative control verified: reverting the fix
fails the test.
… of artifacts

Deep-review fixes for the health detectors, plus a repo-hygiene P0 that this
branch introduced.

**831 MiB of build artifacts were committed here and pushed.** Commit 04cb6da
("read reorg depth from the log") was meant to touch four files; a stray
`git add -A` swept in 2201 files under `contrib/push-relay/target/` — a 79 MB
debug binary, 28 MB of rlibs, and `.rustc_info.json` recording local toolchain
paths. They were unambiguously accidental: the push-relay *source* does not
exist until four PRs later, so this branch carried a compiled binary for a crate
it did not contain. Nothing built or linted those paths, so CI stayed green.
satd squash-merges, so this never entered master's history — but the squashed
*tree* would have, permanently, in a public repo. `git rm -r --cached` here, and
the `/contrib/push-relay/target` ignore rule moves down from #486 to sit ahead
of the crate it protects, since .gitignore cannot untrack what is already
tracked.

**`deep_reorg` was gated on a wall-clock high-water mark, so a backwards clock
step silenced it entirely.** `ReorgWatermark` seeded `SystemTime::now()` at task
start and dropped any record stamped earlier. `ReorgRecord::ts_unix_secs` is
also `SystemTime::now()`, so one NTP correction of a fast RTC after boot — or a
hypervisor resync after live migration, or `date -s` — meant every reorg for the
next N minutes was discarded. The watermark never reset and `deep_reorg` is an
edge, so those alerts were not delayed; they were gone. Replaced with
`ReorgSeen`, a bounded set of `(ts, old_tip, new_tip)` identities that depends on
no clock. It is seeded from the log at startup, which also removes the reason
the watermark existed.

**The 300 s lookback was a second way to lose an edge forever.** Any delay of
this task past the window — API-runtime saturation, a `statvfs` on a hung mount,
a VM pause — dropped older reorgs out of view permanently, with no catch-up
path. The window bought nothing now that de-duplication is exact, so the scan
covers the whole ring (≤256 records).

**`statvfs` now runs on `spawn_blocking`.** `disk_watch_path` defaults to
`blocksdir`, which operators routinely point at NFS/iSCSI, and `statvfs` on a
hung mount blocks uninterruptibly. Inline, it parked an API worker and froze
*every* detector — including the two above — for as long as the mount stayed
wedged.

**The shutdown branch could spin at 100% CPU.** `_ = shutdown.changed() => if
*shutdown.borrow() { return }` never returns once the last sender drops:
`changed()` then fails immediately and forever while `borrow()` still reads
false. Latent in the current wiring, one refactor from real; every other
shutdown handler in the tree returns unconditionally, and now so does this one.

Also: `disk_low` logged an identical WARN every poll (5,760/day while the
condition holds) — now edge-only; the config reference claimed `tip_stall` is
IBD-suppressed (the code deliberately does not suppress it, and says why at
length) and that `alertpeerfloor` defaults to 0 on signet (only regtest);
removed a stale doc paragraph describing a field that no longer exists and a lag
comment whose stated reason was wrong.

Tests: `a_reorg_is_reported_even_when_the_clock_steps_backwards` is
control-verified — reinstating the watermark makes it fail on the missing second
event. Plus rescan-idempotence and startup-seeding coverage.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
A reorg several blocks deep is a routine part of signet and testnet4, not an
incident, and satd was treating it as a permanent error state.

`emit()` mirrored every edge observation into `NodeWarnings`, and
`StatusKind::DeepReorg.severity()` is `Critical`, which maps to
`Severity::Error`. Nothing ever calls `clear` for an event that has no resolved
state — by design, "it happened, and nothing un-happens it" — so the first reorg
at or past `alertreorgdepth` (default 3) put a permanent `Error` in the
registry. From that moment `has_errors()` was true for the life of the process,
`getblockchaininfo.warnings` was never empty again, and the TUI held its
blocking modal open. On a chain where 3-deep reorgs are ordinary that is the
steady state within minutes of starting, and the operator has no way to
acknowledge or clear it.

This contradicts the warnings module's own contract, stated at the top of
`warnings.rs`: warnings "represent *current* state", "history-style events
(reorgs, fee-estimate windows, etc.) have their own persistent logs", and "every
emitted warning indicates a bug that should be fixed". A deep reorg is history,
it has its own persistent log (`ReorgLog` / `getreorghistory`), and it is not a
bug. It is also the exact failure the detectors avoid elsewhere — the
`peer_floor` default is network-conditional specifically so regtest does not
carry "a critical warning that can never clear".

Edge kinds now fire `-alertnotify` without recording anything, via a new
`NodeWarnings::notify_event`. The page still happens on **every** occurrence
(each reorg is a distinct event, not a restatement of one condition), and the
`status` event on the streaming API and webhooks is unchanged — including its
`critical` severity, so a `min_severity = "critical"` hook still receives it.
What goes away is only the standing entry that nothing could clear.

`record_recurring` existed solely to serve this one call site — it opted an id
out of the first-time-only `alertnotify` dedup *because* its warning never
cleared — so it is deleted rather than left as a trap.

Tests are control-verified: reinstating the recording behavior makes
`notify_event_fires_the_hook_without_recording_a_warning` fail on
`count() == 0` (left: 1, right: 0) and `every_occurrence_of_an_event_pages` fail
on the second occurrence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
Depth 3 means completely different things on different chains, and the previous
default paged on all of them.

On mainnet a 3-block reorg costs real hashrate and invalidates transactions
merchants have begun treating as settled; waking someone is right. Signet,
testnet and testnet4 are not economically secured, and reorgs a few blocks deep
are an ordinary consequence of thin, volatile hashrate — so the default ran
`-alertnotify` for the network behaving exactly as designed. An alert that fires
during normal operation is one operators learn to ignore, and ignoring it costs
them the mainnet alert too, which is the one that mattered.

`reorg_depth_for(network)` mirrors the `peer_floor_for(network)` precedent
immediately above it, for the same reason it exists:

- mainnet `3`
- signet / testnet / testnet4 `10` — **raised, not disabled.** Past the
  6-confirmation convention a wallet has been told something false, and that is
  worth reporting on any chain. Disabling would also make the detector's silence
  indistinguishable from health, which is the argument `peer_floor_for` already
  makes for not defaulting signet off.
- regtest `0` — its test suites reorg deliberately and constantly;
  `invalidateblock` and competing-chain tests are the point of the harness.
- an unrecognized future network takes the test-network value: new networks are
  overwhelmingly test networks, and guessing that way fails mild.

This pairs with the previous commit. That one stopped a deep reorg becoming a
permanent `getwarnings` entry; this one stops it paging on chains where it is
routine. Together: on a test network an ordinary reorg now does nothing but emit
a `status` event that subscribers can filter, and a genuinely anomalous one
still reports everywhere.

`health_alert_threshold_defaults_and_parsing` caught the regtest change on its
own and is updated to assert the new intent across all three cases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
`-connect=` pins the node to exactly the addresses named and suppresses
both DNS seeding and the fixed seeds (satd/src/main.rs), so a node given
one or two upstreams can never reach the stock floor of 3. Nothing on the
node would add a peer, so the raise stood forever: in `getwarnings`, in
`-alertnotify`, in the TUI modal, and in `getblockchaininfo.warnings` —
which wallet software renders to end users. `satd_alert_active{kind=
"peer_floor"}` pinned high stops distinguishing peer-starved from
configured-that-way, so the detector carried no signal on exactly the
nodes it was loudest about.

This is the regtest trap in different clothes, and `peer_floor_for` was
already written against it — it just keyed off the network when the
condition it describes is a property of the configuration. So the default
now takes the `-connect` count too, capped at the stock floor.

Capping rather than disabling keeps the alert doing the one useful thing
it still can here: reporting a `-connect=` node that had all its
configured peers and then lost one. An explicit `alertpeerfloor`
overrides in either direction, including to 0.

The default had to move out of the `Config` struct literal, which has
already moved `connect` by the time it reaches that field.

Both new tests were run against the previous behavior and fail there.
Regtest blocks exist only when something calls `generatetoaddress`, so an
idle chain is its resting state rather than a stall. `last_connect` is
seeded at detector start and advanced only by `BlockConnected`, so a node
left running for an hour between test runs raises `tip_stall` — which is
`Critical`, so it pins `getwarnings`, holds `has_errors()` true, and puts
up the TUI's blocking modal, on a chain doing exactly what it should.

This is the third threshold in the set to need it and the last one that
did not have it; `peer_floor_for` and `reorg_depth_for` already turned
regtest off for the same reason, and `peer_floor_for`'s doc comment calls
it "a poor greeting for every developer's first run".

Every other network keeps the hour, test networks included. Unlike a
reorg a few blocks deep, going an hour without a block is not an ordinary
property of thin hashrate — signet and testnet4 are still expected to
make blocks, so relaxing them the way `reorg_depth_for` does would be
answering a different question.

The mainnet raise rate is left alone: at a 600s mean interval,
P(gap > 3600s) = e^-6, so a healthy node raises about every 2.8 days.
That is documented on TIP_STALL_SECS as a deliberate trade and it stays
the operator's to tune.

The regtest assertion in `health_alert_threshold_defaults_and_parsing`
previously asserted 3600 and passed, so it is a direct negative check on
the new value.
`statvfs` on a hard NFS mount whose server has gone away does not
return. `spawn_blocking(..).await` moves the syscall off the detector's
thread but still parks the detector on the JoinHandle, so tip_stall,
deep_reorg, check_mempool and check_peers stopped running, `chain_rx`
stopped being drained, and every gauge froze at its last value — meaning
an external Prometheus rule on tip age could not fire either. A wedged
mount silently disabled the entire alerting subsystem, which is the one
condition alerting exists for. The comment claimed spawn_blocking gave
isolation it does not: it spares the API worker, not the detectors.

A timeout around the join is not enough on its own. `timeout` abandons
the handle but cannot cancel a blocking task, so each poll would strand
another thread and exhaust the bounded blocking pool within hours —
trading a wedged detector for a wedged runtime.

So the probe is now bounded AND carried across polls: each tick awaits
the outstanding handle by `&mut` reference for a 2s budget (well under
the 15s interval, so a filesystem that answers at all is read inline) and
otherwise keeps it for the next tick. One unresponsive mount costs
exactly one stuck thread, forever, and leaves disk_low on its last known
verdict rather than clearing it — an unresponsive mount is not evidence
the disk drained. Logged loudly once, then hourly.

The budget is a parameter so the regression test can use 1ms; the
detector loop passes DISK_PROBE_BUDGET.

Also corrects the three places that said tip_stall is gated on IBD —
the release notes asserted both readings in consecutive sentences, and
config.rs's field doc and CLI help said "outside IBD".
`check_tip_stall_values` deliberately ignores IBD and explains why.

The wedge test was run against the inline await and fails there with
"check_disk must not block on an unresponsive filesystem: Elapsed(())".
@bkeroack
bkeroack force-pushed the feature/a3-health-detectors branch from 7eb8351 to aab93b6 Compare August 2, 2026 00:02
@bkeroack
bkeroack merged commit 43496c6 into master Aug 2, 2026
1 check passed
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.

1 participant