diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 50364adf45..2447f64073 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -90,7 +90,7 @@ def do_check(self, args, repository): raise CommandError("--repair does not allow --max-duration argument.") if args.repair and args.max_age is not None: # repair verifies every pack; reusing recorded results during repair needs repository - # repair (refs #8572). + # repair (refs #10026). raise CommandError("--repair does not allow the --max-age option.") if args.archives_only and args.max_age is not None: # --max-age only affects the repository check; --archives-only skips it. @@ -301,7 +301,7 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): index from the packs if the index is corrupt, provided every pack matches its store hash. If any pack fails its store hash, the repository check leaves the index and the packs untouched and reports it; salvaging the intact objects of - such a pack is not implemented yet (refs #8572). The rebuild authenticates + such a pack is not implemented yet (refs #10026). The rebuild authenticates each object's header and metadata with the key, leaves an object that fails this out of the index and reports it as an error. Repair mode also removes the index entries of the chunks stored in missing packs (packs the index references, diff --git a/src/borg/archiver/compact_cmd.py b/src/borg/archiver/compact_cmd.py index f0c096bfc0..12ecb7f272 100644 --- a/src/borg/archiver/compact_cmd.py +++ b/src/borg/archiver/compact_cmd.py @@ -321,14 +321,17 @@ def compact_packs(self): logger.error(f"{stale_used} of them are still in use: repository data is missing!") set_ec(EXIT_ERROR) - # bytes no index entry covers. compact_pack reclaims the redundant duplicates among them while - # rewriting a pack; reclaiming the rest is tracked in #8572. + # unindexed bytes: pack bytes no index entry covers, e.g. the objects of an interrupted borg create. + # compact_pack reclaims those that are copies of indexed chunks when it rewrites a pack and keeps the + # rest, as "borg check --repair" can recover their objects (#9868). A full check --repair adds these + # objects to the index (--repository-only rebuilds it only if it is corrupt), so the next compact + # reclaims the unused ones. unindexed = sum(total - pack_indexed[pid] for pid, total in pack_total.items() if total > pack_indexed[pid]) if unindexed: logger.info( - f"{format_file_size(unindexed)} in pack files is not covered by the index; " - "redundant copies are reclaimed on pack rewrite, reclaiming the rest is tracked in " - "https://github.com/borgbackup/borg/issues/8572." + f"{format_file_size(unindexed)} in pack files is not covered by the index; copies of indexed " + 'chunks are reclaimed on pack rewrite. Run "borg check --repair" (without --repository-only), ' + 'then "borg compact" to reclaim the rest.' ) # packs recorded corrupt in PackTracker that are still in the store @@ -502,8 +505,8 @@ def build_parser_compact(self, subparsers, common_parser, mid_common_parser): ``borg compact`` reclaims objects the chunk index knows about, plus redundant copies of indexed chunks (e.g. written by concurrent backups) that it finds while rewriting a pack. Other bytes no index entry covers, such as packs left behind by a backup that crashed - before recording its objects, are re-indexed by ``borg check --repair`` and reclaimed by - the next ``borg compact``. + before recording its objects, are re-indexed by ``borg check --repair`` (without + ``--repository-only``) and reclaimed by the next ``borg compact``. ``borg compact`` does not rewrite or merge packs that ``borg check`` recorded as corrupt and warns about them. ``borg check --repair --verify-data`` deletes the corrupt chunks by diff --git a/src/borg/repository.py b/src/borg/repository.py index 2745a9d9f1..c0c1908f9f 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1489,13 +1489,13 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida ArchiveChecker.finish. Packs are verified by the store hash, which is content-addressing rather than a MAC, so that check detects accidental corruption but not tampering; the rebuild therefore checks every object with validate, see below, refs #9901, #10026. If any pack is corrupt the index - is not rebuilt, refs #8572, #10026. Pack ids found corrupt are kept in cache/checked-packs, + is not rebuilt, refs #10026. Pack ids found corrupt are kept in cache/checked-packs, refs #9696. That object is stored in the key's envelope, too, so check() needs the key (see set_key). A pack recorded corrupt fails the check, also on a partial run that stops before re-reaching it. The record clears at the check that finds the pack intact again or gone (removed by - compact; TODO: also when repair salvages and drops it, refs #8572); prune() does this from packs/. + compact; TODO: also when repair salvages and drops it, refs #10026); prune() does this from packs/. It also reports missing packs (refs #9898): pack ids the chunk index references but that are absent from packs/. The index is read from its fragments only and its referenced pack ids are @@ -1790,7 +1790,7 @@ def note_drop(): if repo_only: logger.error( f"{done} {mode} repository check, corrupt pack(s) found{so_far}; repairing a repository " - "with corrupt packs is not implemented yet (refs #8572)." + "with corrupt packs is not implemented yet (refs #10026)." ) else: # a full check's archives phase reads archive/item metadata (and file content with diff --git a/src/borg/testsuite/archiver/compact_cmd_test.py b/src/borg/testsuite/archiver/compact_cmd_test.py index 9f177a4290..a56b8ef5b7 100644 --- a/src/borg/testsuite/archiver/compact_cmd_test.py +++ b/src/borg/testsuite/archiver/compact_cmd_test.py @@ -373,10 +373,10 @@ def test_compact_keeps_unindexed_waste(tmp_path): assert pdchunk(repository.get(H(2))) == b"CCCC" -def test_compact_reclaims_indexed_waste_only(tmp_path): +def test_compact_reclaims_indexed_waste_only(tmp_path, caplog): # compact reclaims a pack's indexed-but-unused bytes, but leaves alone a pack whose only waste is # unindexed (bytes no index entry covers): those may be live data "borg check --repair" can - # recover (#9868). + # recover (#9868). It logs how to reclaim them (#10429). from ...archiver.compact_cmd import ArchiveGarbageCollector location = os.fspath(tmp_path / "repo") @@ -400,8 +400,11 @@ def test_compact_reclaims_indexed_waste_only(tmp_path): gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10) gc.chunks = repository.chunks - gc.compact_packs() + with caplog.at_level(logging.INFO): + gc.compact_packs() + assert "in pack files is not covered by the index" in caplog.text + assert 'Run "borg check --repair" (without --repository-only)' in caplog.text pack_names = [info.name for info in repository.store_list("packs")] # indexed waste -> compacted, kept object still readable assert bin_to_hex(waste_pack) not in pack_names diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 55019ab20c..79599575e5 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1874,7 +1874,7 @@ def validate(chunk_id, obj): def test_check_repair_refuses_when_pack_corrupt(tmp_path): # A repair that finds any corrupt pack leaves the index and the pack untouched (no lossy rebuild, - # nothing dropped) and fails on a repository-only run, refs #8572, #10026. + # nothing dropped) and fails on a repository-only run, refs #10026. location = os.fspath(tmp_path / "repo") with Repository(location, exclusive=True, create=True) as repository: repository.put(H(1), fchunk(b"GOOD-CHUNK", chunk_id=H(1))) @@ -1995,7 +1995,7 @@ def delete_pack(repository, pack_id): def test_check_repair_removes_missing_pack_entries(tmp_path, caplog, repo_only): # a repair removes the index entries of the chunks in a missing pack and stores the index. It fails # a repository-only run, as the chunks are lost; a full check defers them to the archives phase - # (refs #9898, #8572). + # (refs #9898). location = os.fspath(tmp_path / "repo") pack_ids = create_repo_one_pack_per_chunk(location) with Repository(location, exclusive=True) as repository: