check: report missing chunks grouped as chunk -> files -> archives, #9218

[mr-raj12]

Fixes #9218.

borg check logged one line per (archive, file, chunk) for each missing file chunk. Now it groups by chunk:

The following chunks are missing in the repository:
- Chunk a8675a3a8a...95dce, 553,688 bytes
    - var/lib/flatpak/repo/objects/2c/6265...52be.file: archive-2022-03-31, archive-2022-04-29
    - var/lib/flatpak/runtime/.../bin/gpgsm: archive-2022-03-31

For each missing chunk you see its size, the files that reference it, and the archives each file is in. The old per-chunk line (with byte offsets) is now logged at debug level.

The report is collected while checking the archives and printed once they have all been analyzed. As a check can run against a badly damaged repository with very many missing chunks, the collection is bounded by ArchiveChecker.MAX_MISSING_CHUNKS (distinct chunks) and MAX_REFS_PER_CHUNK (files per chunk); the report notes when either limit truncated it.

Tests in check_cmd_test.py cover the grouping (one chunk reported once, listing multiple files and archives) and the truncation.

[codecov]

Codecov Report

❌ Patch coverage is 88.50575% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (d5eb3f7) to head (112cbca).
⚠️ Report is 40 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/borg/archive.py	88.50%	9 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9965      +/-   ##
==========================================
+ Coverage   86.14%   86.18%   +0.03%     
==========================================
  Files          96       96              
  Lines       17326    17397      +71     
  Branches     2649     2668      +19     
==========================================
+ Hits        14925    14993      +68     
- Misses       1663     1664       +1     
- Partials      738      740       +2     

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

[ThomasWaldmann]

So, did you ask claude for a review?

[ThomasWaldmann]

Thanks for the update - the caps, the merged dict, format_file_size and the new grouping test all look good. format_file_size(size) is the right call: with BORG_UNITS in place, BORG_UNITS=raw gives the exact byte count (2160926 B) for people who want it, iec gives MiB.

Some remaining points:

Blocker: the two new truncation tests fail in binary mode

check_cmd_test.py is generated with kinds="local,binary", and the binary_archiver fixture sets EXE="borg.exe" and FORK_DEFAULT=True, so borg runs in a separate process. patch.object(ArchiveChecker, "MAX_MISSING_CHUNKS", 1) in the test process has no effect there, the real caps apply, nothing gets truncated. Reproduced by putting a borg.exe wrapper on PATH:

FAILED check_cmd_test.py::test_missing_file_chunk_report_truncated[binary_archiver]
FAILED check_cmd_test.py::test_missing_file_chunk_refs_truncated[binary_archiver]
E  AssertionError: assert 'only the first 2 files are listed' in '...'
2 failed, 2 passed

The binary variants don't run in normal PR CI (only for tag builds and in the nightly canary workflow), which is why this is green here.

Two cheap fixes:

• test_missing_file_chunk_refs_truncated doesn't need patching at all: create 101 identical files and the real MAX_REFS_PER_CHUNK = 100 fires. Verified against unpatched code - 100 reference lines, then - ... (only the first 100 files are listed).
• test_missing_file_chunk_report_truncated can't do that (it would need 10001 missing chunks), so let it take the archiver fixture directly instead of archivers - the module's pytest_generate_tests only parametrizes when archivers is in the fixture names, so it then runs local-only.

The report is lost on Ctrl-C or an exception

report_missing_chunks() runs only after the archive loop, and the per-chunk line is now logger.debug, so interrupting a long check of a badly damaged repo (or hitting the Unknown archive metadata version raise mid-loop) now prints nothing at all about missing chunks. Before, whatever had been found so far was already in the log.

Please wrap the loop in try/finally, and/or keep one error-level line on the first sighting of each chunk id - deduping by chunk id is exactly what removes the hardlink noise #9218 complained about, and it restores feedback during the run.

The caps bound memory, but not to a small number

MAX_MISSING_CHUNKS * MAX_REFS_PER_CHUNK = up to ~1e6 reference entries. Measured with tracemalloc on that exact structure with ~90 char paths: ~385 MiB when each path is in 1-3 archives, ~2.3 GiB when it is in 24. It is also ~1e6 log lines, i.e. far past what anyone will read.

Since the grouped report is for humans, considerably lower caps (or an extra global cap on the total number of refs kept) would serve both memory and readability. The --debug lines are still there for the full picture.

Truncation should say how much was dropped

... (only the first 10000 missing chunks are listed) doesn't tell whether 10001 or 10 million chunks are missing - probably the most interesting number in a damage report. A plain counter incremented in record_missing_chunk() is enough to print something like 10000 of N missing chunk references listed.

Nit

missing_chunks[chunk_id] = [size, {}] plus entry[1]: a tuple (size, {}) unpacked as size, refs = ... reads better, size is never mutated.