v0.228.0 — the off-site check reads the data; the debug page stops lying (R-399 + R-400)
gates / gates (push) Successful in 12s
gates / gates (push) Successful in 12s
R-399: monitoring.integrity.read_data_subset defaults to 100%. A pack damaged without changing its size made plain `restic check` report "no errors were found" on demo-hp 2026-08-30; every read-data form caught it. Cost on that 134 MB store: 35.0s structure vs 39.2s at 100%. "off" (any case) is the off token; empty means not-configured, therefore the default; a malformed value falls back to the DEFAULT, never to structure. A completed check over 5 minutes logs a WARN naming the duration, the depth and R-401 — operator log only, no hub event, no depth change. The depth is now recorded with the verdict (LastIntegrityDepth; empty = NOT RECORDED, never "structure"). R-400: 24 debug-page references, 17 dispatched, 7 dead — three of which fetched on page LOAD, so those panels were permanently blank. backup/crossdrive implemented; backup/infra, hub/infra-push, dr/infra-status, storage/watchdog-status and both storage/simulate-* deleted with their panels and JavaScript. scripts/debug_route_gate.py fails in both directions and is registered after the seven were resolved. 18 referenced, 18 dispatched, none orphaned. Corrections: the dead-field warning in report/types.go said the controller runs no integrity check and the notifiers are called from nowhere — both false since v0.227.0. controller.yaml.example gains its missing integrity: block. integrityCheckTimeout's "ships OFF" comment rewritten.
This commit is contained in:
@@ -41,14 +41,64 @@ import (
|
||||
// any plausible structure check and far below "forever", so the failure mode of a wedged SFTP mount is
|
||||
// a released flag and a retry tomorrow, never a box whose backups stop because a check never returned.
|
||||
//
|
||||
// It is deliberately NOT sized for a `--read-data-subset` run, which downloads pack data and can take
|
||||
// hours. That option ships OFF (R-399); whoever turns it on must revisit this number, and this comment
|
||||
// is the note that says so.
|
||||
// R-399 CHANGED WHAT THIS NUMBER HAS TO COVER, and this paragraph replaces the one that said the
|
||||
// opposite. Read-data now ships ON at 100% (see defaultIntegrityReadDataSubset below), so a check
|
||||
// downloads and re-hashes the whole store every week. Measured on demo-hp 2026-08-30: 39.2 s at 100%
|
||||
// against a 134 MB store, versus 35.0 s at structure depth. 30 minutes is still ~46x the only
|
||||
// full-depth number that exists, so it is not resized here on the strength of one measurement — but
|
||||
// it is now the number a LARGE store will meet first, and `integritySlowNoticeThreshold` exists to
|
||||
// tell the operator long before that happens. R-401 owns the revisit.
|
||||
const integrityCheckTimeout = 30 * time.Minute
|
||||
|
||||
// integritySlowNoticeThreshold is the point at which a COMPLETED check has started costing real time
|
||||
// and the depth setting needs revisiting (R-401).
|
||||
//
|
||||
// CHOSEN, and deliberately imprecise, from the single data point that exists: demo-hp's 134 MB store,
|
||||
// 2026-08-30, 35.0 s at structure depth and 39.2 s at 100%. 5 minutes is ~7.6x the only full-depth
|
||||
// number we have, so it cannot fire on anything resembling today's fleet; and it is well under
|
||||
// integrityCheckTimeout, so the operator hears "this is getting slow" long before a check is killed
|
||||
// for running too long. A notice changes no behaviour, so an imprecise number is cheap here — whereas
|
||||
// a precise-looking threshold invented from one measurement on one small store would be the exact
|
||||
// shape of the four production designs this project has already specced against nothing.
|
||||
// A var, not a const, for ONE reason: the notice cannot otherwise be proven to fire through the real
|
||||
// CheckOffboxIntegrity path — no test can make a check take five minutes. Tests lower it and restore
|
||||
// it with defer. Nothing in production writes it.
|
||||
var integritySlowNoticeThreshold = 5 * time.Minute
|
||||
|
||||
// defaultIntegrityMaxAgeDays is the max age of a SUCCESSFUL check before one is due again.
|
||||
const defaultIntegrityMaxAgeDays = 7
|
||||
|
||||
// defaultIntegrityReadDataSubset is how deep an unconfigured box checks: ALL of it (R-399, Viktor's
|
||||
// ruling of 2026-08-31).
|
||||
//
|
||||
// THE FACT THE DEFAULT RESTS ON, because it is the thing that stops someone turning it back down to
|
||||
// save four seconds: **the structure check does not detect a size-preserving pack corruption.** On
|
||||
// 2026-08-30 a pack in demo-hp's store was damaged WITHOUT changing its size; plain `restic check`
|
||||
// reported `no errors were found` and exited clean, and every read-data form caught it. A store that
|
||||
// is verified only structurally is a store whose rot is discovered at restore time, with a customer
|
||||
// waiting.
|
||||
//
|
||||
// THE COST, measured the same day on the same store (140 829 678 B / 2 651 blobs / 67 snapshots):
|
||||
// structure 35.0 s, 10% 35.9 s, 50% 37.3 s, 100% 39.2 s. Four seconds.
|
||||
//
|
||||
// WHAT IS NOT ESTABLISHED: how any of that behaves on a store one or two orders of magnitude larger.
|
||||
// There is exactly ONE data point. That is why this is a constant and a notice (see
|
||||
// integritySlowNoticeThreshold) rather than a rotation schedule, a size threshold or a bandwidth
|
||||
// budget — every one of those would be a number invented from a single measurement. R-401.
|
||||
//
|
||||
// This default lives HERE and not in config.applyDefaults, deliberately, and defaultIntegrityMaxAgeDays
|
||||
// beside it is the precedent: both integrity defaults are resolved in this package, in one accessor
|
||||
// each, next to the reasoning that justifies them. Symmetry with the other Monitoring defaults is
|
||||
// worth less than having the number and its argument in the same place.
|
||||
const defaultIntegrityReadDataSubset = "100%"
|
||||
|
||||
// integrityOffToken switches the deep check back off without a code change.
|
||||
//
|
||||
// A setting with no off switch is not a setting. Without this token there would be no way to return a
|
||||
// box to structure depth: an EMPTY value means "not configured" and therefore the default (§8), so
|
||||
// emptiness cannot also mean "off". Matched case-insensitively.
|
||||
const integrityOffToken = "off"
|
||||
|
||||
// readDataSubsetRe accepts the forms restic documents for --read-data-subset: "n/m", a percentage
|
||||
// like "5%", or a size like "50M". Anything else is refused at read time rather than passed through —
|
||||
// a typo must not fail the whole check, which is what handing restic an unparsed value would do.
|
||||
@@ -80,21 +130,64 @@ type IntegrityResult struct {
|
||||
|
||||
// integrityReadDataSubset resolves the configured subset spec, refusing anything malformed.
|
||||
//
|
||||
// Fail-safe direction: an unrecognised value becomes "" (structure check only) with a WARN, never a
|
||||
// passthrough. Handing restic `--read-data-subset=banana` fails the entire check, which would turn a
|
||||
// typo in a config file into a store that silently stops being verified.
|
||||
// FOUR inputs, three outcomes (§8's table):
|
||||
// - absent or empty -> defaultIntegrityReadDataSubset. Empty is "not configured", never "off".
|
||||
// - "off" (any case) -> "" , the structure-and-index check only. The one way to switch it back.
|
||||
// - a form restic accepts -> itself, unchanged. An explicit value always wins.
|
||||
// - anything else -> defaultIntegrityReadDataSubset, with a WARN naming the bad value.
|
||||
//
|
||||
// THE MALFORMED CASE FALLS BACK TO THE DEFAULT, NOT TO STRUCTURE, and the direction is the point.
|
||||
// Handing restic `--read-data-subset=banana` fails the whole check, so a typo must not be passed
|
||||
// through — but downgrading to structure depth on a typo would ALSO silently remove the protection
|
||||
// R-399 exists to add, which is R-357's shape exactly: a guard that opens quietly. Falling back to the
|
||||
// default keeps the protection and still says loudly that the config is wrong.
|
||||
func (m *Manager) integrityReadDataSubset() string {
|
||||
spec := strings.TrimSpace(m.cfg.Monitoring.Integrity.ReadDataSubset)
|
||||
if spec == "" {
|
||||
return defaultIntegrityReadDataSubset
|
||||
}
|
||||
if strings.EqualFold(spec, integrityOffToken) {
|
||||
return ""
|
||||
}
|
||||
if !readDataSubsetRe.MatchString(spec) {
|
||||
m.logger.Printf("[WARN] [offbox] integrity: read_data_subset %q is not a form restic accepts (n/m, N%%, or a size like 50M) — running the STRUCTURE check only", spec)
|
||||
return ""
|
||||
m.logger.Printf("[WARN] [offbox] integrity: read_data_subset %q is not a form restic accepts (n/m, N%%, a size like 50M, or %q) — falling back to the DEFAULT depth %q, not to a structure-only check, so a typo cannot quietly remove the protection",
|
||||
spec, integrityOffToken, defaultIntegrityReadDataSubset)
|
||||
return defaultIntegrityReadDataSubset
|
||||
}
|
||||
return spec
|
||||
}
|
||||
|
||||
// IntegrityDepthCode is the depth as a short RECORDED value, for the persisted verdict and the wire.
|
||||
//
|
||||
// "" is reserved to mean NOT RECORDED — a box older than v0.228.0, whose stored verdict cannot say how
|
||||
// deep it looked. That follows the StatsKnown precedent on the same object: absence means "cannot
|
||||
// answer", never an answer. So structure depth is written as the word "structure", not as "".
|
||||
func IntegrityDepthCode(subset string) string {
|
||||
if subset == "" {
|
||||
return "structure"
|
||||
}
|
||||
return subset
|
||||
}
|
||||
|
||||
// noticeIfSlow logs an operator WARN when a COMPLETED check has started costing real time (R-401).
|
||||
//
|
||||
// COMPLETED ONLY. A skip has no duration to judge, and an unreachable store is "I could not look",
|
||||
// which is not "I looked and it was slow" (§8). Pass and fail BOTH qualify: the notice and the failure
|
||||
// alarm are independent facts and neither suppresses the other.
|
||||
//
|
||||
// It is a log line and NOTHING else — no hub event, no customer alarm. An event type costs the
|
||||
// severity contract, the grain table and three registers, all to say "this took a while"; 08 §6.2's
|
||||
// coarse-by-default rule points the other way. And it does NOT change the depth by itself: a notice
|
||||
// that silently reconfigures the box would be a behaviour change wearing a notice's clothes.
|
||||
func (m *Manager) noticeIfSlow(res IntegrityResult) {
|
||||
if res.Skipped || res.Unreachable || res.Duration < integritySlowNoticeThreshold {
|
||||
return
|
||||
}
|
||||
m.logger.Printf("[WARN] [offbox] integrity: the check took %s at depth %s (%s), over the %s notice threshold — R-401: the depth setting needs revisiting for a store this size. Nothing was changed automatically.",
|
||||
res.Duration.Round(time.Second), IntegrityDepthCode(res.ReadDataSubset),
|
||||
integrityDepthLabel(res.ReadDataSubset), integritySlowNoticeThreshold)
|
||||
}
|
||||
|
||||
// integrityMaxAge returns the configured max age of a successful check, defaulting to 7 days.
|
||||
func (m *Manager) integrityMaxAge() time.Duration {
|
||||
d := m.cfg.Monitoring.Integrity.MaxAgeDays
|
||||
@@ -133,16 +226,29 @@ func (m *Manager) IntegrityDue(now time.Time) (due bool, last time.Time) {
|
||||
// the hourly operator cooldown already governs the mail, and the failure is already recorded where a
|
||||
// surface can read it. A skip or an unreachable repository does NOT reach here, so tomorrow tries
|
||||
// again.
|
||||
// RecordIntegrityOutcome is the exported entry point; the caller in main.go owns the decision of WHEN
|
||||
// a verdict counts, because only it knows whether the run was forced or scheduled.
|
||||
func (m *Manager) RecordIntegrityOutcome(at time.Time, ok bool) { m.recordIntegrityOutcome(at, ok) }
|
||||
// RecordIntegrityVerdict is the PRODUCTION entry point: it persists the verdict AND the depth it was
|
||||
// reached at, in one write. The caller in main.go owns the decision of WHEN a verdict counts, because
|
||||
// only it knows whether the run was forced or scheduled.
|
||||
//
|
||||
// The depth travels with the verdict because a stored result that does not say how deep it looked
|
||||
// cannot be judged later: "checked, OK" means two different things at structure depth and at 100%,
|
||||
// and the whole of R-399 is that difference.
|
||||
func (m *Manager) RecordIntegrityVerdict(res IntegrityResult) {
|
||||
m.recordIntegrityOutcome(res.RanAt, res.OK, IntegrityDepthCode(res.ReadDataSubset))
|
||||
}
|
||||
|
||||
func (m *Manager) recordIntegrityOutcome(at time.Time, ok bool) {
|
||||
// RecordIntegrityOutcome records a verdict whose depth is not stated. It writes "" to the depth field,
|
||||
// which reads as NOT RECORDED rather than as structure depth — see integrityDepthCode. Kept as the
|
||||
// due-ness surface the R-359 tests drive; production goes through RecordIntegrityVerdict above.
|
||||
func (m *Manager) RecordIntegrityOutcome(at time.Time, ok bool) { m.recordIntegrityOutcome(at, ok, "") }
|
||||
|
||||
func (m *Manager) recordIntegrityOutcome(at time.Time, ok bool, depth string) {
|
||||
if err := m.settings.UpdateOffboxStatus(func(o *settings.OffboxTarget) {
|
||||
o.LastIntegrityCheck = at.UTC().Format(time.RFC3339)
|
||||
o.LastIntegrityOK = ok
|
||||
o.LastIntegrityDepth = depth
|
||||
}); err != nil {
|
||||
m.logger.Printf("[ERROR] [offbox] integrity: could not persist the check outcome: %v — the check RAN and its verdict was ok=%v, but due-ness did not advance, so it will run again tomorrow", err, ok)
|
||||
m.logger.Printf("[ERROR] [offbox] integrity: could not persist the check outcome: %v — the check RAN and its verdict was ok=%v at depth %q, but due-ness did not advance, so it will run again tomorrow", err, ok, depth)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -198,6 +304,7 @@ func (m *Manager) CheckOffboxIntegrity(ctx context.Context) IntegrityResult {
|
||||
if err == nil {
|
||||
res.OK = true
|
||||
m.logger.Printf("[INFO] [offbox] integrity: check PASSED in %s (%s)", res.Duration.Round(time.Second), integrityDepthLabel(res.ReadDataSubset))
|
||||
m.noticeIfSlow(res)
|
||||
return res
|
||||
}
|
||||
|
||||
@@ -221,6 +328,9 @@ func (m *Manager) CheckOffboxIntegrity(ctx context.Context) IntegrityResult {
|
||||
}
|
||||
|
||||
m.logger.Printf("[ERROR] [offbox] integrity: check FAILED after %s — restic reported: %s", res.Duration.Round(time.Second), res.Output)
|
||||
// A slow FAILING check gets the notice too. The two facts are independent and suppressing one
|
||||
// because the other fired is how the second fact stops existing.
|
||||
m.noticeIfSlow(res)
|
||||
return res
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user