check: keep pack check results, add --max-age to reuse them

[mr-raj12]

Refs #9696.

cache/checked-packs maps pack id -> (timestamp, result) and holds the results of the repository pack check. Before this branch its contents were throwaway: a completed check called tracker.clear(), which empties the table and deletes the store object, and the next check cleared it again before starting. The records existed only to let a --max-duration check resume where the previous one stopped.

That made two things impossible. A cheap cron check could not hand its findings to a later repair, so corrupt pack ids only ever appeared as log lines in whichever run happened to find them. And a check could not reuse a result that was still perfectly good, which matters on remote or cloud storage where hashing every pack takes hours.

This branch keeps the records instead and lets the user decide, at check time, how old a result may be.

Retention. Records are kept in all cases. tracker.clear() at the end of a check becomes tracker.prune(pack_ids): drop the records whose pack id is no longer listed in packs/, then store the rest. Pruning runs after the pack listing has been scanned, on both the completed and the interrupted path.

Reuse. The new borg check --max-age INTERVAL skips a pack when its recorded result is intact and younger than INTERVAL. Without --max-age (the default) every pack is verified, and the results are still recorded. Records of corrupt packs are always re-verified regardless of age, so a repair never acts on a stale failure. --max-age cannot be combined with --repair.

Partial checks. --max-duration no longer has a private progress mechanism of its own: progress now comes from the record timestamps, so it requires --max-age and errors out without it. This is a small breaking change for anyone passing bare --max-duration, and it is the one point in this branch worth your explicit sign-off. In exchange the two options compose into the behaviour you described: with --max-duration=3600 --max-age=1w, a daily one hour check verifies packs that have no result younger than a week, until the whole repository has been covered once per week.

The summary names the corrupt packs, guarded on index_errors == 0 so the ids only appear when the pack check actually ran.

No format change and no version bump. Old blobs load unchanged, and an older borg reading a newer blob sees result=0 entries it re-verifies anyway. A single store object was preferred over a separate cache/corrupt-packs, because result already encodes the distinction and two files can drift out of sync where one cannot.

docs/internals/data-structures.rst describes the retention rule and how --max-age and --max-duration interact.

Tests in repository_test.py and check_cmd_test.py cover: intact and corrupt records surviving a completed check; a plain check recording results that a later --max-age check reuses; a check without --max-age verifying every pack while keeping the records; a carried-over corrupt record being re-verified and replaced once the pack is intact; selective pruning of records whose pack is no longer listed; corrupt records surviving across consecutive partial checks; the summary naming the corrupt ids; and the --repair/--max-age and --max-duration/--max-age argument rules. black and ruff are clean.

Not in here: machine-readable output, --repair consuming the recorded list, clearing records after a successful repair. Those depend on how borg check --repair and a separate repair command end up being split, which is still open.

[codecov]

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.52%. Comparing base (e5ed8f6) to head (e7676d1).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/borg/repository.py	95.65%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9925      +/-   ##
==========================================
+ Coverage   85.49%   85.52%   +0.03%     
==========================================
  Files          93       93              
  Lines       16122    16139      +17     
  Branches     2466     2473       +7     
==========================================
+ Hits        13783    13803      +20     
+ Misses       1632     1630       -2     
+ Partials      707      706       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[ThomasWaldmann]

I think we could keep all entries except the ones for packs that do not exist any more.

See also #9898.

The condition for skipping a pack would then change from "it is in checked-packs AND it was OK" to "it is in checked-packs AND it was OK AND the result is recent enough".

[mr-raj12]

I think we could keep all entries except the ones for packs that do not exist any more.

See also #9898.

The condition for skipping a pack would then change from "it is in checked-packs AND it was OK" to "it is in checked-packs AND it was OK AND the result is recent enough".

Implemented as a new --max-age INTERVAL option (e.g. 4w). Skip condition is now: in checked-packs AND it was OK AND the record is younger than max_age. Intact records are kept across cycles, only records of packs no longer listed in packs/ are pruned at cycle end. Corrupt records are always re-verified.
Default is 0 (current behaviour, full check verifies every pack) - can change it to a non-zero default if you prefer.

[ThomasWaldmann]

CI: /home/runner/work/borg/borg/docs/misc/asciinema/README.rst: WARNING: document isn't included in any toctree [toc.not_included]

I'll fix that, was a collateral damage of my asciinema updates.

Update: #9938 merged, please rebase on current master.

[ThomasWaldmann]

review by claude opus 5:

Reviewed at e7676d18d (correctly rebased on current master e846a85df). I built the branch in a throwaway worktree and ran repository_test.py (77 passed) and check_cmd_test.py (22 passed, 2 skipped) — clean. Both findings below were confirmed by running probe tests against the branch.

The design is sound and matches what was asked for in the earlier comment: retention is unconditional, the skip condition became "recorded AND ok AND recent enough", pruning is by "no longer listed in packs/". The prune() placement after the for/else is correct on both paths, because pack_infos is the complete listing taken up front — an interrupted partial check still prunes safely.

Two real defects, then smaller stuff.

---

1. Partial check reports "Corrupt packs" and "no problems found", exit code 0

src/borg/repository.py:856

if index_errors == 0:  # the packs were checked, so the corrupt records are from this check

That comment is false on the --max-duration break path. A partial check that stops before reaching a pack recorded corrupt by an earlier run still emits that pack's id, while objs_errors is 0. Confirmed output from a two-pack repo where the recorded-corrupt pack sorts second and max_duration expires after the first:

INFO  Checked 1 index files (0 errors) and 1 packs (0 errors).
ERROR Corrupt packs: 6442040b644f009de79ec6352c535f22fffb254a0a4efcfb0107cd39a1b2b1bd
INFO  Finished partial repository check, no problems found.

check() returned True, so no EXIT_WARNING. This is exactly the advertised workflow (daily --max-duration=3600 --max-age=1w cron): a corruption found on Monday gets re-announced at ERROR level on every subsequent day that doesn't reach it, each time with exit code 0 and "no problems found". A cron job keyed on the exit code sees success; a human reading the log sees a contradiction.

Two coherent ways out, and I'd take the second:

• Report only what this run verified — accumulate the corrupt ids in a local list next to pack_errors instead of asking the tracker.
• Keep reporting all recorded corrupt ids, but make the wording and the outcome agree: say which ones were re-verified this run vs. carried over, and let carried-over ones count toward the failure result. A partial check that knows about an unrepaired corrupt pack arguably should keep exiting non-zero until it is repaired — that is the point of persisting the record. It does mean a partial check fails forever until repair exists, which is a maintainer call.

Either way pack_errors == 0 and corrupt_ids should not produce "no problems found".

2. A future timestamp makes a pack unverifiable forever

src/borg/repository.py:827

if max_age and time.time() - entry.timestamp < max_age:

If entry.timestamp is in the future the difference is negative, which is < max_age for any max_age, so the pack is skipped — and skipped again on every later check until wall clock catches up. timestamp is stored as "Q", so a client with a badly wrong clock (or a repo checked from several hosts, one of them skewed) can write a record that suppresses verification of that pack for years. Confirmed with a record 10 years ahead and max_age=1: the pack was not hashed.

One-line fix, treating a future record as stale:

age = time.time() - entry.timestamp
if max_age and 0 <= age < max_age:
    continue

The failure here is silent rather than noisy, which is what makes it worse than the usual clock-skew annoyances.

---

Smaller points

--max-duration now requiring --max-age. Explicit sign-off was asked for, so: the hard error looks right to me, rather than a silent behaviour change. Old semantics genuinely cannot be reproduced — the old cycle reset is gone, so a bare --max-duration would restart from pack 0 every run and never finish. Two things would soften it: make the message actionable ("--max-duration requires --max-age, e.g. --max-age=4w" — the current one leaves the user to find the docs), and consider whether defaulting --max-age to something like 4w when --max-duration is passed would be better than breaking existing cron lines outright.

--archives-only --max-age=… is silently ignored (check_cmd.py:71 — the repo check is skipped entirely). --max-duration errors out in the same situation (check_cmd.py:52). Worth the matching CommandError for consistency.

The summary hides how much was skipped. With --max-age, Checked N index files (0 errors) and M packs (0 errors) reports M < total with no hint why, and then says "no problems found" — a user could reasonably read that as a full verification. Adding the reused count, e.g. … and M packs (0 errors, K results reused), makes the reduced coverage visible.

Unbounded corrupt-id log line (repository.py:859). A badly damaged repo joins every corrupt id into one line; 10k packs is a ~650 KB log record. Cap it (first ~100 plus a count) — the full list is on record in checked-packs anyway.

Nested condition reads oddly now (repository.py:826-828). Without max_age the outer if does nothing; it's one condition:

if entry is not None and entry.result and max_age and 0 <= time.time() - entry.timestamp < max_age:
    continue  # recorded intact recently; corrupt records are always re-verified

Stale test name. test_check_full_ignores_recorded_set no longer describes what it asserts — the comment was updated, the name wasn't. Something like test_check_without_max_age_verifies_all_but_keeps_records.

Doc wording nit, docs/internals/data-structures.rst: "pruned when a check finishes scanning packs/" — pruning also happens on the interrupted path, which is correct behaviour but not what the sentence says. "when a check finishes" is enough.

Not a PR issue: docs/usage/check.rst.inc doesn't list --max-age, but that is the periodic build_usage/build_man commit, not something to hand-edit here.

[ThomasWaldmann]

For 2. add and use borg.constants.MAX_CLOCK_SKEW = 7200 # [s] - everything that is more than that in the future is considered invalid and re-checked.

corrupt-id log line: 1 id per line?