feat(#490): opt-in fail-closed miner hold on unrecoverable dashboard health failures#708
Merged
Merged
Conversation
…health failures Adds dashboard.fail_closed (default false). Off, an unrecoverable failure only alerts (existing #131/#337 paths) and mining continues, since the dashboard is an observability layer independent of the xmrig-proxy -> p2pool -> monerod datapath. On, it reuses the #35 sync gate's own hold (stop/start SYNC_GATE_CONTAINERS) rather than a new path, but — unlike that one-way latch — re-checks every cycle and releases once the condition clears. Scoped narrowly to genuine, non-transient failures: the DB auto-heal (#489) rebuild itself failing (StateManager.is_db_unrecoverable, distinct from the existing db_healthy flag, which also covers an ordinary transient write error that must never gate), or the dashboard container crash-looping (ContainerHealthMonitor.is_bad("dashboard")). XvB/payout state-integrity loss already safes to plain p2pool via algo_service's existing fail_count>=3 guard and isn't duplicated here. Registered in config.reference.json and both drift guards (pithead's CONTROL_DASHBOARD_EDITABLE_KEYS, control_service.py's EDITABLE_ENV_KEY_PATHS) so it round-trips the #33 control-approval path the same as dashboard.tari_required. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sync gate and the new fail-closed gate ponytail-review finding: _apply_sync_gate and _apply_fail_closed_gate duplicated the same stop-every-container / start-every-container-tracking-ok loops. Extract _stop_gate_containers(quiet) / _start_gate_containers() and use them from both, so the two holds over the same container set can't drift out of step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…a first-sighting seed
Coordinator finding 1: is_bad("dashboard") read the raw level, which
ContainerHealthMonitor.update() seeds to "bad" on a first-sighting unhealthy
snapshot WITHOUT clearing the 120s continuous-unhealthy debounce a known
container must pass. That seed is intentionally silent for alerting, but the
fail-closed gate read it as a live truth, so a dashboard reported unhealthy at
first sighting could hold the fleet on an unconfirmed, possibly-transient state
— contradicting the "only confirmed, non-transient failures gate" intent.
Fix: rename is_bad -> is_confirmed_bad and require c["alerted"] (True only when
the debounce produced an alert edge — crash loop or unhealthy past the
threshold), so a silently-seeded baseline never gates. One-line predicate
change; the alerted flag already existed.
Finding 2: added classification-layer tests pinning transient-never-gates —
seed not confirmed, single/early unhealthy not confirmed before 120s, confirmed
only after the debounce (test_container_health); the disabled-sinks alert path
still feeds the tracker and confirms through the 120s debounce
(test_alert_service); and the real trigger expression
(is_db_unrecoverable OR is_confirmed_bad) built from real objects does not gate
on a seed (test_data_service). The is_db_unrecoverable-only-after-failed-rebuild
and transient-write-error-does-not vs. gate cases were already covered in
test_storage_service.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6 tasks
- CHANGELOG: open the [Unreleased] section (consumed into [1.10.0] at the last release-prep) with the dashboard.fail_closed Added entry, matching the per-feature-PR convention #596/#597 followed. - testing-strategy.md: the shipped accessor is is_confirmed_bad, not is_bad; also mark tier-3 deferred (· 3 ▶) to match the sibling docker-control stop/start rows and the PR's own deferred-to-gouda note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d branch — union CHANGELOG + dashboard.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #490
Summary
Adds
dashboard.fail_closed(default false).db_unhealthy,db_reset,container_unhealthy) and shows a dashboard badge; mining continues. The dashboard is an observability layer — the mining datapath (xmrig-proxy→p2pool→monerod) is independent of it, so a cosmetic dashboard fault must never idle the fleet.fail_closed: true: on an unrecoverable failure, holdsp2pool+xmrig-proxyuntil it clears, by reusing the Don't start p2pool and xmrig proxy until after monero and tari have synced fully #35 sync-gate's own mechanism (docker_control.stop/startoverSYNC_GATE_CONTAINERS) — no new hold path. Unlike the sync gate's one-way latch, this re-checks every cycle and releases on its own once the condition clears (an operator fix rarely needs a full stack restart). It only engages once the sync gate has already released the miner.The exact "unrecoverable" set, and why
Scoped deliberately narrow — false positives here idle rigs and cost revenue:
StateManager.is_db_unrecoverable(), a new flag set only when_recover_corrupt_db's own rebuild attempt raises (disk full, permissions). This is distinct from the existingdb_healthyflag, which also flips false on an ordinary transient write error (a lock, a momentary I/O hiccup) — that case must alert only, never gate, and now provably doesn't (test_transient_write_error_flags_unhealthy_without_reset).ContainerHealthMonitor.is_bad("dashboard")(newis_bad()accessor on the existing Telegram alert: container crash-loop / unhealthy #337 tracker), read live rather than re-derived from edges.Deliberately excluded per the issue's own interaction note: XvB/payout state-integrity loss.
algo_service's existingfail_count >= 3guard already safes that to plain P2Pool (#9/#70) regardless offail_closed— duplicating a hold there would be redundant and riskier.Wiring
config.reference.json+ both drift guards (pithead'sCONTROL_DASHBOARD_EDITABLE_KEYS,control_service.py'sEDITABLE_ENV_KEY_PATHS) registerdashboard.fail_closedthe same waydashboard.tari_requiredis — a normaldashboard.*toggle round-tripping the existing Stack config editor: change any setting from the dashboard, applied via pithead (incl. P2Pool mode hot-swap) #33 control-approval path.docker-compose.ymlpassesDASHBOARD_FAIL_CLOSEDthrough to the dashboard container (same pattern asTARI_REQUIRED).AlertService.process()'s disabled-sinks early return now also feeds the container-health tracker (previously only fed when a sink was enabled) — otherwise a Telegram-off,fail_closed-on stack would never see the dashboard's own crash-loop state.Miner held (fail-closed)badge, distinct from the existingMiner held (sync)badge.docs/configuration.md,docs/dashboard.md(the trade-off, in house voice),docs/architecture.md(Docker Control's use list),docs/dev/testing-strategy.md(scenario catalog row).Ponytail review
Ran
/ponytail-reviewon the diff. One finding:_apply_sync_gateand the new_apply_fail_closed_gateduplicated the same stop-every-container / start-every-container loops. Fixed by extracting shared_stop_gate_containers(quiet)/_start_gate_containers()helpers used by both, so the two holds overSYNC_GATE_CONTAINERScan't drift out of step (net -4 lines in a follow-up commit). No other findings — no new abstractions, no speculative config, no unused flexibility.Test status (all run locally, not claimed from memory)
make test-dashboard: 1481 passed, coverage 96.71% (gate: ≥80%)make test-frontend: 222 passed (includes theconfiglogic.mjsLOGICAL_GROUPS completeness check for the newdashboard.fail_closedschema path)make test-stack(shell suite): 1549 passed, 0 failed — includes the newDASHBOARD_FAIL_CLOSEDrender-default/propagation,describe_changeINFO-not-DEST, and editable-allowlist round-trip casesbash tests/stack/test_compose.sh: pass (docker-compose.yml env passthrough + hardening invariants)bash tests/integration/selftest.sh: 132 passed, 0 failedtests/integration/fakes): 23 passeddiff-covervsorigin/develop, which matches the base commitdevelop-v1.11currently shares): 100% (gate: ≥90%)make lint: every target passes exceptlint-proto, which fails only because this sandbox has no Docker daemon (buf lint/buf buildneeddocker run) — pre-existing environment gap, unrelated to this change (no.protofiles touched).Deferred
Tier-3/4 coverage (real containers actually stopping/starting on the fail-closed path, a real crash-looping dashboard container) — host-affecting, so per standing process it needs a tier-4 gouda pass before merging to
develop, same as other Docker-Control-driven features.Targets the v1.11 integration branch; merges to develop after v1.10 ships + a tier-4 gouda pass (fail-closed hold is host-affecting).
🤖 Generated with Claude Code