diff --git a/src/borg/archive.py b/src/borg/archive.py index 541e69e2d2..23b5245616 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2312,7 +2312,8 @@ def check( # each index object's store hash, and the index is the authoritative record of which chunks exist, # so we do not rebuild it from the packs (reading every pack is far too slow for a routine check). # --repair does rebuild from the packs (slow_rebuild=repair), working from the real packs so it - # can detect and fix archives that reference chunks whose pack has gone missing. + # can detect and fix archives that reference chunks whose pack has gone missing. It also replaces a + # corrupt index, see Repository.check. # The rebuild validates every object header it walks, because a corrupt data_size parses fine # and points the walk into the middle of the pack. That costs one metadata slot read and one # decryption per object and it needs the key, so read the key here if we do not have it yet. diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 50364adf45..f0d7bb1f47 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. @@ -119,7 +119,7 @@ def do_check(self, args, repository): max_duration=args.max_duration, max_age=max_age, repo_only=args.repo_only, - # the object validator for the index rebuild, which only a repair does. + # validates each object the index rebuild of a --repository-only repair walks. validate=object_validator(RepoObj(key)), ): set_ec(EXIT_WARNING) @@ -297,16 +297,18 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): In practice, repair mode hooks into both the repository and archive checks: - 1. When checking the repository's consistency, repair mode rebuilds the repository - 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 - 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, - but that are absent from the repository). Only a full ``borg check --repair`` - repairs the archives that reference these chunks, ``--repository-only`` does not. + 1. When checking the repository's consistency, repair mode verifies every pack if + the index is corrupt. A full ``borg check --repair`` then rebuilds the index from + the packs in the archive check (which does so on every ``--repair`` run). With + ``--repository-only``, the repository check rebuilds it, provided every pack + matches its store hash. If any pack fails its store hash, it leaves the index and + the packs untouched and reports it; salvaging the intact objects of such a pack + is not implemented yet (refs #10026). Either 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, but that are + absent from the repository). Only a full ``borg check --repair`` repairs the + archives that reference these chunks, ``--repository-only`` does not. A missing or corrupt repository defaults object is replaced by empty defaults, so the repository can be used again; the commands then use the built-in defaults. diff --git a/src/borg/archiver/compact_cmd.py b/src/borg/archiver/compact_cmd.py index f0c096bfc0..383ef8fdea 100644 --- a/src/borg/archiver/compact_cmd.py +++ b/src/borg/archiver/compact_cmd.py @@ -321,14 +321,21 @@ 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. + # superseded duplicates are unindexed objects whose chunk id the index maps to another location. + # compact_pack drops those when it rewrites a pack and keeps the other unindexed objects, as + # "borg check --repair" can recover them (#9868). A full check --repair (--repository-only rebuilds + # the index only if it is corrupt) indexes one copy per chunk id, so objects whose chunk id had no + # index entry become reclaimable once unused, and the other copies stay superseded duplicates. + # TODO(#10471): count superseded duplicates as reclaimable, so compact rewrites a pack that holds only + # used objects and superseded duplicates, e.g. after a crashed borg create was re-run. 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. " + '"borg check --repair" (without --repository-only) indexes the objects whose chunk id has no ' + 'index entry, so "borg compact" reclaims them once unused. Copies of chunks indexed elsewhere ' + "are only reclaimed when compact rewrites their pack." ) # packs recorded corrupt in PackTracker that are still in the store @@ -502,8 +509,11 @@ 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 kept. ``borg check --repair`` (without + ``--repository-only``) indexes one copy per chunk id, so ``borg compact`` reclaims the + objects whose chunk id had no index entry once they are unused. The remaining copies of + chunks indexed elsewhere (e.g. when the crashed backup was re-run) stay unindexed and are + only reclaimed when ``borg compact`` rewrites their pack to reclaim unused indexed objects. ``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..8d24b9f949 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1469,7 +1469,7 @@ def info(self): info = dict(id=self.id, version=self.version) return info - def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, validate=None): + def check(self, repair=False, max_duration=0, max_age=0, repo_only=None, validate=None): """Check repository consistency. packs/ and index/ objects are named by the store hash of their content, so a pack or index @@ -1484,18 +1484,18 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida rebuild re-reads every pack anyway - so a read-only check just stops and reports it instead of continuing. A read-only check never rebuilds the index: reading every pack to do so would be far too slow and expensive for a routine (e.g. cron) check. With repair=True and a corrupt - index, and if every pack is intact, the index is rebuilt from the packs' object headers and - persisted; on a full check the archives phase rebuilds and re-persists it afterwards, see - 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, - refs #9696. That object is stored in the key's envelope, too, so check() needs the key (see - set_key). + index, every pack is verified. With repo_only, and if every pack is intact, the index is then + rebuilt from the packs' object headers and persisted. Without repo_only, the archives phase + rebuilds and persists it (see ArchiveChecker.check and ArchiveChecker.finish), refs #10434. 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. With repo_only, if any pack is corrupt, the index 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 @@ -1514,11 +1514,12 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida max_age, accepting a future timestamp up to MAX_CLOCK_SKEW (clock skew). Results are recorded regardless of max_age. - repo_only: whether this is a repository-only run. In repair mode it sets the return value for - damage repair does not fix, i.e. a corrupt pack, a missing pack or a skipped pack byte range (see - validate): fail if repo_only, else defer (a full check's archives phase can repair a corrupt pack - holding metadata, or file content with --verify-data, and reports and repairs the archives that - reference chunks the index lacks). + repo_only: whether this is a repository-only run. Required if repair. In repair mode, if True, a + corrupt index is rebuilt here (see above), and damage repair does not fix, i.e. a corrupt pack, a + missing pack or a skipped pack byte range (see validate), fails the check. If False, both are left + to the archives phase: it rebuilds the index, can repair a corrupt pack holding metadata (or file + content with --verify-data), and reports and repairs the archives that reference chunks the index + lacks. validate: validate(chunk_id, obj) -> bool, True if obj (an object's header plus its metadata slot) is the repo object with id chunk_id, see repoobj.object_validator. Required if repair. @@ -1528,6 +1529,7 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida none. Each skipped range counts as one error. """ assert validate is not None or not repair + assert repo_only is not None or not repair def verify(namespace, name): # name is the store hash of the object's content, so it is intact iff store.hash() matches. @@ -1597,11 +1599,15 @@ def store_list(namespace): # --repair forbids --max-duration and --max-age, so the partial and max_age handling in # the loop stays inactive during a repair. packs_scanned = True - if index_errors: + if index_errors and repo_only: logger.warning( "Repository index is corrupted; verifying all packs before deciding whether to " "rebuild it from them." ) + elif index_errors: + logger.warning( + "Repository index is corrupted; verifying all packs, the archives check rebuilds the index." + ) # packs are the bulk of the work and the part --max-duration spreads over several checks. pack_infos = store_list("packs") # drop objects whose name is not a valid pack name and count them as errors; the code @@ -1715,10 +1721,17 @@ def recorded_ts(info): logger.info("Finished checking packs.") tracker.prune(present_pack_ids) pack_pi.finish() - # rebuild only on repair, if the index was the sole problem and every pack was verified intact - # this run: sig_int breaks the loop early, so "no pack errors" must be paired with "all packs - # scanned" (pack_files == len(pack_infos)) to not rebuild from unverified packs. - if repair and index_errors and pack_errors == 0 and not sig_int and pack_files == len(pack_infos): + # rebuild only on a repository-only repair, if the index was the sole problem and every pack was + # verified intact this run: sig_int breaks the loop early, so "no pack errors" must be paired with + # "all packs scanned" (pack_files == len(pack_infos)) to not rebuild from unverified packs. + if ( + repair + and repo_only + and index_errors + and pack_errors == 0 + and not sig_int + and pack_files == len(pack_infos) + ): def note_drop(): nonlocal drops @@ -1737,14 +1750,16 @@ def note_drop(): interruptible=True, ) except ChunkIndexRebuildInterrupted: - # nothing was stored: the corrupt fragments stay, so the next use rebuilds from the packs. + # nothing was stored: the corrupt fragments stay. drops = 0 # counted by the discarded rebuild, which covered only a part of the packs - logger.warning("Index rebuild interrupted; the index stays corrupt and is rebuilt on next use.") + logger.warning('Index rebuild interrupted; the index stays corrupt, run "borg check --repair".') else: self.invalidate_chunk_index() # the rebuilt index is persisted; drop the in-memory copy index_repaired = True else: logger.error("Repository index is corrupted and must be repaired; skipping the pack check.") + # index_deferred: the archives phase rebuilds the corrupt index; it runs only if this check was not interrupted. + index_deferred = bool(index_errors) and repair and not repo_only and not sig_int objs_errors = index_errors + pack_errors + len(missing_pack_ids) + drops summary = ( f"Checked {index_files} index files ({index_errors} errors) " @@ -1790,20 +1805,23 @@ 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 # --verify-data), so it repairs a corrupt pack holding such objects; warn rather than fail. - logger.warning(f"{done} {mode} repository check, corrupt pack(s) found{so_far}.") + deferred = "; index corrupt, the archives check rebuilds it from the packs" if index_deferred else "" + logger.warning(f"{done} {mode} repository check, corrupt pack(s) found{deferred}{so_far}.") elif drops: - # a full check's archives phase reports the chunks the archives reference but the index - # lacks, so warn only. - log = logger.error if repo_only else logger.warning - log( + # drops come from a repository-only rebuild only; no archives phase follows it. + logger.error( f"{done} {mode} repository check, " f"index rebuilt without pack byte range(s) it could not authenticate{so_far}." ) + elif index_deferred: + logger.warning( + f"{done} {mode} repository check, index corrupt; the archives check rebuilds it from the packs." + ) elif index_errors and not index_repaired: # the index is corrupt but was not rebuilt, e.g. the pack verification was interrupted # before every pack was confirmed intact; the corrupt index is left in place. @@ -1817,11 +1835,10 @@ def note_drop(): else: # missing packs: the archives phase repairs the archives that reference their chunks. logger.warning(f"{done} {mode} repository check, missing pack(s) found{so_far}.") - # in repair mode a corrupt index left unrebuilt is a failure; a corrupt or missing pack, or a - # skipped pack byte range, fails only a repository-only run, while a full check defers it to the - # archives phase. + # in repair mode, a corrupt index neither rebuilt here nor deferred fails; a corrupt or missing pack, + # or a skipped pack byte range, fails a repository-only run, a full check defers it to the archives phase. if repair: - if index_errors and not index_repaired: + if index_errors and not index_repaired and not index_deferred: return False return not (repo_only and (pack_errors or corrupt_ids or missing_pack_ids or drops)) return not problems diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 90a03f873f..5258eb9124 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -23,7 +23,7 @@ ) from ...crypto.key import RepositoryKeyInfoMissing from ...constants import * # NOQA -from ...helpers import bin_to_hex, CommandError, CorruptPack, Error, sig_int +from ...helpers import bin_to_hex, hex_to_bin, CommandError, CorruptPack, Error, sig_int from ...helpers import BackupDamagedChunksError from ...helpers.passphrase import PassphraseWrong from ...hashindex import ChunkIndex @@ -908,10 +908,18 @@ def test_check_format_missing_archive_metadata(archivers, request): assert "Analyzing archive archive2" in output # the intact archive still uses the given format -def test_check_repair_rebuilds_corrupt_index(archivers, request): - # A corrupt index with all packs intact: the default (full) --repair rebuilds the index from the - # packs and persists it (via the archives check, see ArchiveChecker.finish), leaving the repository - # usable again without a slow rebuild on the next access. +@pytest.mark.parametrize( + "mode, message", + [ + ([], "the archives check rebuilds it from the packs"), + (["--repository-only"], "Repository index was corrupted and has been rebuilt from the packs."), + ], + ids=["full", "repository-only"], +) +def test_check_repair_rebuilds_corrupt_index(archivers, request, mode, message): + # A corrupt index with all packs intact: --repair rebuilds the index from the packs and persists it, + # leaving the repository usable again. A full check rebuilds it in the archives check (see + # ArchiveChecker.finish), a repository-only check in the repository check. archiver = request.getfixturevalue(archivers) check_cmd_setup(archiver) cmd(archiver, "check", exit_code=0) @@ -929,8 +937,8 @@ def test_check_repair_rebuilds_corrupt_index(archivers, request): else: with pytest.raises(CorruptChunkIndexFragment): cmd(archiver, "check") - output = cmd(archiver, "check", "-v", "--repair", exit_code=0) - assert "rebuilt" in output.lower() + output = cmd(archiver, "check", "-v", "--repair", *mode, exit_code=0) + assert message in output # item 6: repair persisted a fresh index instead of leaving it for a slow rebuild on the next # access. confirm the on-disk index exists and every fragment is intact. archive, repository = open_archive(archiver.repository_path, "archive1") @@ -943,6 +951,96 @@ def test_check_repair_rebuilds_corrupt_index(archivers, request): assert "archive1" in cmd(archiver, "repo-list") # and remains usable +def test_check_repair_rebuilds_corrupt_index_with_corrupt_pack(archivers, request): + # A corrupt index and a corrupt pack: a full --repair rebuilds and stores the index in the archives check. + # The corrupt pack stays recorded corrupt, so a following check still fails on it, refs #10434. + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + with open_repository(archiver) as repository: + bad_pack = sorted(info.name for info in repository.store_list("packs"))[0] + name = f"packs/{bad_pack}" + repository.store_store(name, corrupt(repository.store_load(name), -1)) + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + repository.store_store(name, corrupt(repository.store_load(name), 0)) + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + assert "corrupt pack(s) found; index corrupt, the archives check rebuilds it from the packs." in output + with open_repository(archiver) as repository: + index_infos = list(repository.store_list("index")) + assert index_infos # a fresh index was stored + for info in index_infos: # each fragment's content matches its store hash name + assert repository.store.hash(f"index/{info.name}", algorithm=STORE_HASH_NAME) == info.name + output = cmd(archiver, "check", "-v", exit_code=1) + assert "Checked 1 index files (0 errors)" in output + assert f"Corrupt pack: {bad_pack}" in output + + +def test_check_repair_walks_packs_once_for_corrupt_index(archiver, monkeypatch): + # A full --repair with a corrupt index walks the objects of each pack once, for the index rebuild in the + # archives check, refs #10434. + # local-only: this patches PackReader in-process. + check_cmd_setup(archiver) + with Repository(archiver.repository_path, exclusive=True) as repository: + pack_ids = {hex_to_bin(info.name) for info in repository.store_list("packs")} + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + repository.store_store(name, corrupt(repository.store_load(name), 0)) + + orig_iter_headers = PackReader.iter_headers + walks = [] + + def counting_iter_headers(self, **kwargs): + walks.append(self.pack_id) + return orig_iter_headers(self, **kwargs) + + monkeypatch.setattr(PackReader, "iter_headers", counting_iter_headers) + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + monkeypatch.setattr(PackReader, "iter_headers", orig_iter_headers) + # finish() also walks the packs the repair wrote, so count the packs that existed before the check only. + assert sorted(pack_id for pack_id in walks if pack_id in pack_ids) == sorted(pack_ids) + assert "the archives check rebuilds it from the packs" in output + cmd(archiver, "check", exit_code=0) # the stored index is intact and matches the packs + + +def test_check_repair_interrupt_during_corrupt_index_rebuild(archiver, monkeypatch): + # A full --repair rebuilds a corrupt index in the archives check. A Ctrl-C during that rebuild stops the + # check and stores nothing: the corrupt fragments stay, a command needing the index aborts, and a second + # --repair completes, refs #10434. + # local-only: this patches PackReader in-process. + check_cmd_setup(archiver) # produces several packs + with Repository(archiver.repository_path, exclusive=True) as repository: + assert len(repository.store_list("packs")) > 1 # there is a pack boundary to stop at + for info in repository.store_list("index"): # rot every index fragment + name = f"index/{info.name}" + repository.store_store(name, corrupt(repository.store_load(name), 0)) + index_before = {info.name for info in repository.store_list("index")} + + orig_iter_headers = PackReader.iter_headers + packs_read = [] + + def iter_headers_then_interrupt(self, **kwargs): + packs_read.append(self.pack_id) + yield from orig_iter_headers(self, **kwargs) + sig_int._sig_int_triggered = True # one Ctrl-C after the first pack was walked + + monkeypatch.setattr(PackReader, "iter_headers", iter_headers_then_interrupt) + try: + with pytest.raises(Error, match="Got Ctrl-C"): + cmd(archiver, "check", "--repair") + finally: + sig_int._sig_int_triggered = False # reset the global flag for the following tests + # restore the real method; monkeypatch.undo() would also drop the autouse env (BORG_TESTONLY_WEAKEN_KDF). + monkeypatch.setattr(PackReader, "iter_headers", orig_iter_headers) + assert len(packs_read) == 1 # the repository check walked no pack, the archives check stopped after one + with Repository(archiver.repository_path, exclusive=True) as repository: + assert {info.name for info in repository.store_list("index")} == index_before # nothing stored + + with pytest.raises(CorruptChunkIndexFragment): + cmd(archiver, "repo-list") # the index is still corrupt; commands needing it abort + cmd(archiver, "check", "--repair", exit_code=0) + cmd(archiver, "check", exit_code=0) + + def tamper_object_keeping_pack_name(repository): """Flip a byte in the metadata slot of the 2nd object of a pack holding more than 2 objects, and store the pack under the store hash of its new content, so its content still matches its name. Corrupt @@ -1136,19 +1234,28 @@ def test_find_lost_archives_skips_chunk_with_corrupt_object_header(archivers, re assert "Archive consistency check complete, problems found." in output -def test_check_repair_validates_index_rebuild(archivers, request): - """--repair leaves an object that fails validation out of the index and keeps the object after it (#9901).""" +@pytest.mark.parametrize("repo_only", [False, True], ids=["full", "repository-only"]) +def test_check_repair_validates_index_rebuild(archivers, request, repo_only): + """--repair leaves an object that fails validation out of the index and keeps the object after it (#9901). + + A full check rebuilds the index in the archives check, a repository-only check in the repository check. + """ archiver = request.getfixturevalue(archivers) if archiver.get_kind() != "local": pytest.skip("inspects the store directly") check_cmd_setup(archiver) with KeyedRepository(archiver.repository_location, exclusive=True) as repository: tampered_id, neighbour_id = tamper_object_keeping_pack_name(repository) - output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + if repo_only: + output = cmd(archiver, "check", "-v", "--repair", "--repository-only", exit_code=EXIT_WARNING) + assert "index rebuilt without pack byte range(s) it could not authenticate" in output + else: + output = cmd(archiver, "check", "-v", "--repair", exit_code=0) + assert "the archives check rebuilds it from the packs" in output + assert "Archive consistency check complete, problems found." in output assert "does not authenticate" in output - assert "continuing at the object at offset" in output - assert "index rebuilt without pack byte range(s) it could not authenticate" in output - assert "Archive consistency check complete, problems found." in output + # the index is rebuilt once, so the tampered object is reported once. + assert output.count("continuing at the object at offset") == 1 with KeyedRepository(archiver.repository_location) as repository: assert tampered_id not in repository.chunks assert neighbour_id in repository.chunks diff --git a/src/borg/testsuite/archiver/compact_cmd_test.py b/src/borg/testsuite/archiver/compact_cmd_test.py index 9f177a4290..527f7e4a1f 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 what a full "borg check --repair" does about 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 '"borg check --repair" (without --repository-only) indexes the objects' 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..976a9a946c 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -18,7 +18,7 @@ from ..compress import CNONE from ..constants import MAX_CLOCK_SKEW, ROBJ_FILE_STREAM from ..crypto.key import AESOCBKey, AuthenticatedKey, Blake3AuthenticatedKey, CHPOKey -from ..helpers import Error, IntegrityError, Location, bin_to_hex +from ..helpers import Error, IntegrityError, Location, bin_to_hex, hex_to_bin from ..hashindex import ChunkIndex, ChunkIndexEntry from ..platform import get_process_id from ..repository import Repository, MAX_DATA_SIZE, MAX_VALIDATED_META_SIZE, propagate_rsh, rest_serve_command @@ -1810,7 +1810,7 @@ def test_check_reports_invalid_pack_name(tmp_path, caplog): def test_check_repair_rebuilds_corrupt_index(tmp_path, caplog): - # check(repair=True) rebuilds a corrupt index from the packs' object headers. + # check(repair=True, repo_only=True) rebuilds a corrupt index from the packs' object headers. location = os.fspath(tmp_path / "repo") ids = [H(x) for x in range(10)] with Repository(location, exclusive=True, create=True) as repository: @@ -1828,7 +1828,8 @@ def test_check_repair_rebuilds_corrupt_index(tmp_path, caplog): with reopen(repository) as repository: caplog.clear() with caplog.at_level(logging.INFO, logger="borg.repository"): - assert repository.check(repair=True, validate=validate_any) is True # repair rebuilds the index + # repair rebuilds the index + assert repository.check(repair=True, repo_only=True, validate=validate_any) is True # each rotted fragment is counted once (the cross-check does not load the known corrupt index). assert f"Checked {len(index_names)} index files ({len(index_names)} errors)" in caplog.text with reopen(repository) as repository: @@ -1837,10 +1838,47 @@ def test_check_repair_rebuilds_corrupt_index(tmp_path, caplog): assert pdchunk(repository.get(cid)) == bytes([i]) * 20 # every chunk is indexed and resolves -@pytest.mark.parametrize("repo_only", [True, False]) -def test_check_repair_rebuild_validates_objects(tmp_path, caplog, repo_only): - # check(repair=True, validate=...) does not index an object validate rejects and reports it, refs - # #9901. That fails a repository-only run only. +def test_check_full_repair_leaves_corrupt_index_to_archives_phase(tmp_path, caplog, monkeypatch): + # check(repair=True, repo_only=False) with a corrupt index verifies every pack, walks no pack objects, + # stores no index and succeeds: the archives phase rebuilds the index, refs #10434. + location = os.fspath(tmp_path / "repo") + with Repository(location, exclusive=True, create=True) as repository: + for x in range(3): + repository.put(H(x), fchunk(b"DATA-%02d" % x, chunk_id=H(x))) + repository.flush() # seal a separate pack per chunk + with reopen(repository) as repository: + for info in repository.store_list("index"): # rot every fragment + name = f"index/{info.name}" + data = bytearray(repository.store_load(name)) + data[0] ^= 0xFF + repository.store_store(name, bytes(data)) + orig_iter_headers = PackReader.iter_headers + walked = [] + + def counting_iter_headers(self, **kwargs): + walked.append(self.pack_id) + return orig_iter_headers(self, **kwargs) + + monkeypatch.setattr(PackReader, "iter_headers", counting_iter_headers) + with reopen(repository) as repository: + pack_ids = {hex_to_bin(info.name) for info in repository.store_list("packs")} + index_before = {info.name for info in repository.store_list("index")} + with caplog.at_level(logging.INFO, logger="borg.repository"): + assert repository.check(repair=True, repo_only=False, validate=validate_any) is True + assert "and 3 packs (0 errors)." in caplog.text # every pack was verified + assert "Finished full repository check, index corrupt; the archives check rebuilds it" in caplog.text + assert "has been rebuilt" not in caplog.text + assert walked == [] # no pack object was walked + assert {info.name for info in repository.store_list("index")} == index_before # nothing stored + tracker = PackTracker.load(repository) + assert {pack_id for pack_id in pack_ids if tracker.get(pack_id).result} == pack_ids # recorded intact + with reopen(repository) as repository: + assert repository.check(repair=False) is False # the index is still corrupt + + +def test_check_repair_rebuild_validates_objects(tmp_path, caplog): + # check(repair=True, repo_only=True, validate=...) does not index an object validate rejects, reports it + # and fails, refs #9901. location = os.fspath(tmp_path / "repo") ids = [H(x) for x in range(10)] rejected_id = ids[4] @@ -1862,7 +1900,7 @@ def validate(chunk_id, obj): caplog.set_level(logging.INFO) with reopen(repository) as repository: - assert repository.check(repair=True, repo_only=repo_only, validate=validate) is not repo_only + assert repository.check(repair=True, repo_only=True, validate=validate) is False assert set(ids) <= set(validated) assert "skipped 1 pack byte range(s) it could not authenticate" in caplog.text with reopen(repository) as repository: @@ -1874,7 +1912,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))) @@ -1900,9 +1938,42 @@ def test_check_repair_refuses_when_pack_corrupt(tmp_path): assert repository.check(repair=False) is False # index was not rebuilt; still corrupt -def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatch): +def test_check_full_repair_defers_corrupt_index_with_corrupt_pack(tmp_path, caplog): + # check(repair=True, repo_only=False) with a corrupt index and a corrupt pack succeeds and stores no + # index: the archives phase rebuilds the index and repairs what the corrupt pack held, refs #10434. + 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))) + repository.flush() # seal a pack holding H(1) + repository.put(H(2), fchunk(b"LOST-CHUNK", chunk_id=H(2))) + repository.flush() # seal a separate pack holding H(2) + with reopen(repository) as repository: + bad_pack_id = repository.chunks[H(2)].pack_id + bad_pack_name = "packs/" + bin_to_hex(bad_pack_id) + data = bytearray(repository.store_load(bad_pack_name)) + data[-1] ^= 0xFF # rot the pack holding H(2): its content no longer matches its store hash name + repository.store_store(bad_pack_name, bytes(data)) + for info in repository.store_list("index"): # rot every fragment + name = f"index/{info.name}" + idata = bytearray(repository.store_load(name)) + idata[0] ^= 0xFF + repository.store_store(name, bytes(idata)) + with reopen(repository) as repository: + index_before = {info.name for info in repository.store_list("index")} + with caplog.at_level(logging.INFO, logger="borg.repository"): + assert repository.check(repair=True, repo_only=False, validate=validate_any) is True + assert "and 2 packs (1 errors)." in caplog.text + assert "corrupt pack(s) found; index corrupt, the archives check rebuilds it from the packs." in caplog.text + assert {info.name for info in repository.store_list("index")} == index_before # nothing stored + assert bad_pack_name in [f"packs/{info.name}" for info in repository.store_list("packs")] # not dropped + assert PackTracker.load(repository).corrupt_ids() == [bad_pack_id] # recorded corrupt + + +@pytest.mark.parametrize("repo_only", [True, False]) +def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatch, repo_only): # an interrupted repair (SIGINT before every pack is verified) must not rebuild the index from - # packs it did not confirm intact: it leaves the corrupt index in place and fails. + # packs it did not confirm intact: it leaves the corrupt index in place and fails. A full check fails, too: + # an interrupted check skips the archives phase. location = os.fspath(tmp_path / "repo") ids = [H(x) for x in range(10)] with Repository(location, exclusive=True, create=True) as repository: @@ -1919,7 +1990,7 @@ def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatc monkeypatch.setattr("borg.repository.sig_int", True) # simulate a SIGINT before the pack loop with caplog.at_level(logging.ERROR, logger="borg.repository"): # interrupted: index not rebuilt, so it fails - assert repository.check(repair=True, validate=validate_any) is False + assert repository.check(repair=True, repo_only=repo_only, validate=validate_any) is False assert "index still corrupt" in caplog.text with reopen(repository) as repository: assert repository.check(repair=False) is False # repair left the index corrupt @@ -1966,7 +2037,7 @@ def iter_headers_then_interrupt(self, **kwargs): assert len(repository.store_list("packs")) > 1 # there is a pack boundary to stop at index_before = set(info.name for info in repository.store_list("index")) with caplog.at_level(logging.WARNING, logger="borg.repository"): - assert repository.check(repair=True, validate=validate) is False + assert repository.check(repair=True, repo_only=True, validate=validate) is False assert "Index rebuild interrupted" in caplog.text assert "Interrupted full repository check, index still corrupt so far." in caplog.text assert "index rebuilt" not in caplog.text @@ -1995,7 +2066,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: @@ -2207,7 +2278,7 @@ def test_check_repairs_index_fragment_failing_authentication(tmp_path, caplog, t assert repository.check(repair=False) is False assert f"Store object index/{name} is corrupted" in caplog.text assert name in {info.name for info in repository.store_list("index")} # a check does not write. - assert repository.check(repair=True, validate=accept_all) is True + assert repository.check(repair=True, repo_only=True, validate=accept_all) is True assert name not in {info.name for info in repository.store_list("index")} assert repository.check(repair=False) is True