fix(backup): reconcile-missing is about one server, not its group - #478
Merged
Conversation
backup-reconcile-missing was filed group-scoped, on the reasoning that a device lying about a successful backup should page regardless of the server's monitoring state. Scope was the wrong lever for that: group issues key on (server_group_id, source, ref), so every server in a group collided on one row. One server's failure overwrote another's message, and the recovery arm — which cleared on the *group* issue being active — let a healthy server file the Passed that cleared a broken server's alert, then re-raise it next sweep. It is now an ordinary server-scoped check with the same ref, subject to the monitoring gate like the other per-server backup signals, matching what restore-verification already does. Stored silences carry over: the ref is unchanged and the scoped-policy chain consults the server's group as well as the server. Alert text also named servers by uuid. staleness.rs already had server_label(); reconcile.rs now shares it, resolved in one query for the whole scan. The report-gap and size-mismatch messages were fetching the Server and then printing server.id anyway — they name it properly now too. The migration folds live group-scoped rows onto the server recorded in their check detail (or, for rows predating detail-carrying filings, the uuid in their message text), keeping incident membership intact. Rows with no server to fold onto can no longer be reached by any code path, so they are released from their incident and resolved rather than left holding it open forever.
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
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.
🤖 A group-wide
backup-reconcile-missingalert that names a single server — by uuid — turned out to be two visible symptoms of one mistake, plus a third defect neither symptom exposed.The mistake
backup-reconcile-missingwas filed atScope::Group, on the reasoning that a device lying about a successful backup should page regardless of the server's monitoring state. Scope was the wrong lever for that.Group issues key on
(server_group_id, source, ref), and the ref folds in only the backup type — not the server. So every server in a group collided on one issue row:Passedthat cleared a broken server's alert — and the next sweep re-raised it. Order-dependent flapping.The spec justified group scope for signals "that belong to no single server", but this one names a server in its own message.
restore-verificationhad the same tension and was already resolved the other way: a fleet-stable ref carrying the extra dimensions, filed at server scope.Changes
backup-reconcile-missingbecomes an ordinary server-scoped check with the same ref. Stored silences carry over untouched — the ref is unchanged, andCheckPolicy::apply_scopedconsults the server's group as well as the server, so an existing group-level silence still matches.All three reconcile messages now use
server_label()(shared out ofstaleness.rs), resolved in one batched query for the whole scan rather than per finding. Report-gap and size-mismatch were the worse offenders: both fetched theServerand then interpolatedserver.idanyway.Spec: missing-snapshot reconciliation moves to the per-server bullet, and
restore-verificationis dropped from the group-level list where it was already stale. Two servers failing the same check now hold separate alerts, and alert text names servers the way an operator knows them. Stale doc comments inrefs.rs,restore.rs, andissues.rscorrected to match.Behaviour change
Reconcile-missing now respects the monitoring gate. The alternative was an
always_pageflag onCheckFilingto decouple "always pages" from "is a group fact", butbackup-stalenessandbackup-neverare equally data-safety signals and are already gated — a server whose backups you have deliberately stopped monitoring should not page on any of them.Migration
2026-08-03-233339-0000_fold_reconcile_missing_to_server_scopefolds live group-scoped rows onto the server recorded in their check detail, falling back to the uuid in the message text for rows predating detail-carrying filings. Incident membership survives, so an open incident stays continuous and the next sweep rewrites its message with the server's name.Rows with no server to fold onto (the server was deleted) can no longer be reached by any code path, so they would hold their incident open forever. Those are released from the incident and resolved, and an incident left with no failing contributors is closed — mirroring the leave path in
re_evaluate_incident_membership, without enqueueing a Slack resolve for servers that no longer exist.Alerts the collision hid are not recoverable and are not recreated; the next sweep files them fresh against their own servers.
Verified against a populated database in a rolled-back transaction: rows folded via both attribution paths, orphan released and resolved.
Incidental
A one-line
unused_mutwarning fix intests/it/incident_list_status.rs, already present on main, rode along in the same commit.