diff --git a/docs/internals/data-structures.rst b/docs/internals/data-structures.rst index a2187bfa33..cb4bd9603a 100644 --- a/docs/internals/data-structures.rst +++ b/docs/internals/data-structures.rst @@ -90,9 +90,11 @@ cache/ of an unchanged archive. chunkindex-invalid a marker object: while it is present, the chunks index in ``index/`` is considered - invalid. It is written before deleting index fragments and removed after the last - stale fragment is gone, so an interrupted run does not leave the remaining - fragments looking like a complete index. + invalid, because its fragments may be missing entries or point at deleted packs. + It is written before deleting index fragments, before a single-object delete removes + the old pack, and before ``borg check --repair`` rebuilds the index after changing + the packs. It is removed after the last fragment is deleted or once the complete + current index is stored. Note that this ``cache/`` namespace is inside the repository (and thus shared by all clients); it is not the client-local cache described in diff --git a/docs/internals/packs.rst b/docs/internals/packs.rst index 4774d7ff42..3269948a71 100644 --- a/docs/internals/packs.rst +++ b/docs/internals/packs.rst @@ -322,11 +322,14 @@ and deletes all the fragments it supersedes. A deletion that could drop entries -- dropping the index entirely, or the full rewrite above -- is guarded by a marker object, ``cache/chunkindex-invalid``, written before -the first deletion and removed after the last one. While the marker is present, -leftover fragments could be an incomplete index, so they are not merged; the index is -rebuilt from the pack files on the next load instead. A consolidation needs no marker: -the entries of the small fragments it deletes are already contained in the merged -fragments it wrote before deleting them. +the first deletion and removed after the last one. A single-object delete writes the +marker just before it removes the old pack, and ``borg check --repair`` writes it +before rebuilding the index after changing the packs; both remove it once the index +is stored. While the marker is present, the fragments may be missing entries or point +at deleted packs, so they are not merged; the index is rebuilt from the pack files on +the next load instead. A consolidation needs no marker: the entries of the small +fragments it deletes are already contained in the merged fragments it wrote before +deleting them. If the entire ``index/`` namespace is lost or corrupt, the ChunkIndex can be rebuilt by scanning pack files directly; see :ref:`pack-recovery`. diff --git a/src/borg/archive.py b/src/borg/archive.py index da266d6258..3401f6e1de 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -23,6 +23,7 @@ from . import xattr from .chunkers import get_chunker, Chunk, release_chunk_data from .cache import ChunkListEntry, build_chunkindex_from_repo, write_chunkindex_to_repo +from .cache import write_chunkindex_invalid, delete_chunkindex_invalid from .crypto.key import key_from_repository from .constants import * # NOQA from .digests import ContentDigester @@ -2265,7 +2266,7 @@ def 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. - # manifest_only=True: the other key source make_key uses is self.chunks, built just below. + # manifest_only=True: self.chunks, the other key source of make_key, is set up below. if repair and self.key is None: self.key = self.make_key(repository, manifest_only=True) if self.key is not None: @@ -2274,25 +2275,35 @@ def check( validate = object_validator(self.repo_objs) else: validate = None - self.chunks = build_chunkindex_from_repo( - self.repository, - slow_rebuild=repair, - validate=validate, - # dropped content is a check finding, with or without --repair. - on_drop=self.note_dropped_objects, - # without a validator the rebuild can not resync past a corrupt object header. --repair - # drops the rest of that pack to get on with the repair; without --repair the rebuild - # raises, so an index missing objects that are still there can not make the check report - # them as gone. - drop_corrupt_tail=repair, - write_immediately=False, - ) - # repository.chunks is a separate index, lazily built when repository.get() resolves a - # chunk location. It walks the same packs, so give it the same corrupt-header handling the - # rebuild above got - otherwise the check aborts at a header it just resynced past, halfway - # through its diagnosis. Dropping the rest of that pack stays a --repair action. - self.repository.chunkindex_validate = validate - self.repository.chunkindex_drop_corrupt_tail = repair + # store the chunks buffered in the pack writer, so the index below has their pack locations + # (pack id, offset and size in the pack). + self.repository.flush() + if not repair and self.repository.is_chunk_index_loaded: + # without --repair, use the loaded index. + self.chunks = self.repository.chunks + else: + # free the loaded index first, so only one index is in memory. --repair builds it from the packs. + self.repository.invalidate_chunk_index() + self.chunks = build_chunkindex_from_repo( + self.repository, + slow_rebuild=repair, + validate=validate, + # dropped content is a check finding, with or without --repair. + on_drop=self.note_dropped_objects, + # without a validator the rebuild can not resync past a corrupt object header. --repair + # drops the rest of that pack to get on with the repair; without --repair the rebuild + # raises, so an index missing objects that are still there can not make the check report + # them as gone. + drop_corrupt_tail=repair, + write_immediately=False, + ) + # clear F_NEW (entry not in the index/ fragments yet), so Repository.close() does not store + # this index; finish() stores it with --repair. Without --repair, a repository without index/ + # fragments keeps none. With the invalid marker set (see write_chunkindex_invalid), the build + # deletes the fragments. + self.chunks.clear_new() + # get(), put() and delete() use the repository's index. + self.repository.chunks = self.chunks if self.key is None: self.key = self.make_key(repository) self.repo_objs = RepoObj(self.key) @@ -2420,12 +2431,11 @@ def verify_data(self): ) except IntegrityErrorBase: # failed twice -> remove this defect chunk. delete rewrites its pack without it, - # keeping the other chunks. update_index=False: finish() rebuilds the index from - # the rewritten packs anyway, so a per-chunk full index write would be wasted. + # keeping the other chunks, and removes it from self.chunks, so rebuild_archives + # reports the file it belongs to. update_index=False: finish() stores the index + # rebuilt from the packs and clears the invalid marker delete() writes. self.repository.delete(defect_chunk, update_index=False, validate=validate) self.chunks_modified = True - # drop it from our own index too, so rebuild_archives reports the file it belongs to. - del self.chunks[defect_chunk] else: logger.warning("chunk %s not deleted, did not consistently fail.", bin_to_hex(defect_chunk)) else: @@ -2563,19 +2573,20 @@ def record_missing_chunk(archive_name, path, chunk_id, size): def add_callback(chunk): id_ = self.key.id_hash(chunk) - cdata = self.repo_objs.format(id_, {}, chunk, ro_type=ROBJ_ARCHIVE_STREAM) + cdata = None + if self.repair and id_ not in self.chunks: + # cdata: the compressed and encrypted chunk, which only add_reference stores. + cdata = self.repo_objs.format(id_, {}, chunk, ro_type=ROBJ_ARCHIVE_STREAM) add_reference(id_, len(chunk), cdata) return id_ def add_reference(id_, size, cdata): - # either we already have this chunk in repo and chunks index or we add it now - if id_ not in self.chunks: + # size: unused, part of the archive_put_items callback signature. + # with --repair, store a chunk the repository does not have; put() adds it to self.chunks. + if self.repair and id_ not in self.chunks: assert cdata is not None - self.chunks.add(id_, size) - if self.repair: - pack_results = self.repository.put(id_, cdata) - self.chunks.update_pack_info(pack_results) - self.chunks_modified = True + self.repository.put(id_, cdata) + self.chunks_modified = True def verify_file_chunks(archive_name, item): """Verify that all of a file's chunks are present, collecting any missing ones for the report.""" @@ -2797,9 +2808,12 @@ def finish(self): # writer buffer (close() requires an empty buffer, #10055) before we (re)build the index. self.repository.flush() if self.chunks_modified: - # the packs changed, so the index no longer matches them: rebuild it from the packs - # and persist it: deleting a defect chunk rewrites its pack and repoints that - # pack's other objects in the repository's index, so our offsets for them are stale. + # the packs changed: rebuild the index from them and store it. The index/ fragments lack + # the chunks this repair stored, so the index is invalid until the rebuilt one is stored. + # Free the current index first, so only one index is in memory. + write_chunkindex_invalid(self.repository) + self.repository.invalidate_chunk_index() + self.chunks = None logger.info("Rebuilding and writing the repository chunks index.") build_chunkindex_from_repo( self.repository, @@ -2814,6 +2828,8 @@ def finish(self): write_chunkindex_to_repo( self.repository, self.chunks, incremental=False, clear=False, force_write=True, delete_other=True ) + # the stored index matches the packs: clear the invalid marker. + delete_chunkindex_invalid(self.repository) # drop the in-memory index so close() does not persist it over the index just written. self.repository.invalidate_chunk_index() self.manifest.write() diff --git a/src/borg/cache.py b/src/borg/cache.py index 50807d075a..e038e607c8 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -619,15 +619,21 @@ def chunkindex_is_invalid(repository): def write_chunkindex_invalid(repository): - """Mark the chunk index as invalid. Call before deleting index fragments. + """Store the invalid marker, cache/chunkindex-invalid. - If the deletion is interrupted, the marker remains and the index is rebuilt on next load. + Store it before deleting index/ fragments whose entries no other fragment holds, before deleting a pack + the fragments point at, and before rebuilding the index after pack changes the fragments do not record. + While it is present, build_chunkindex_from_repo rebuilds the index from the packs instead of merging the + fragments. """ repository.store_store(f"cache/{CHUNKINDEX_INVALID_SENTINEL}", b"") def delete_chunkindex_invalid(repository): - """Clear the chunk-index-invalid marker. Call after all fragment deletions have completed.""" + """Delete the invalid marker, if present. + + The index/ fragments, if any, must hold the complete current index and point only at existing packs. + """ try: repository.store_delete(f"cache/{CHUNKINDEX_INVALID_SENTINEL}") except StoreObjectNotFound: @@ -648,7 +654,7 @@ def delete_chunkindex_from_repo(repository): pass if hashes or invalid: # clear the marker after every fragment is gone; also clears a marker left behind by an - # earlier interrupted deletion. + # interrupted operation. delete_chunkindex_invalid(repository) logger.debug(f"chunk indexes deleted: {hashes}") # the in-memory index is now stale; drop it so close() does not write it back into the @@ -921,7 +927,7 @@ def build_chunkindex_from_repo( if chunkindex_is_invalid(repository): if fragments_only: return None - # leftover fragments may be incomplete or stale. Finish the interrupted deletion + # the fragments may be missing entries or point at deleted packs. Delete them # (best-effort; a read-only client rebuilds in memory only), then rebuild from packs. logger.warning("chunk index is invalid (interrupted operation), rebuilding it.") try: diff --git a/src/borg/constants.py b/src/borg/constants.py index 9baac146ad..8bfc3499f6 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -137,11 +137,11 @@ # How often to restart merging the fragments into a chunk index when a listed fragment vanishes # mid-merge (a concurrent repack replaced it). After that, fall back to the slow rebuild from packs. CHUNKINDEX_MERGE_ATTEMPTS = 3 -# Marker object in the cache/ namespace: the chunk index is invalid. Written before deleting index -# fragments, removed after the last fragment is gone. While it is present, the chunk index is rebuilt -# from the packs on next load and the leftover index/ fragments are deleted. Removing the marker while -# index/ fragments remain makes those fragments look like a complete index, so only -# delete_chunkindex_invalid() removes it, and clearing cache/ requires clearing index/ too. +# Marker object in the cache/ namespace: the chunk index is invalid. While it is present, the index/ fragments +# may be missing entries or point at deleted packs, so the chunk index is rebuilt from the packs on next load +# and the leftover fragments are deleted. +# Removing the marker while index/ fragments remain makes those fragments look like a complete index, so +# only delete_chunkindex_invalid() removes it, and clearing cache/ requires clearing index/ too. CHUNKINDEX_INVALID_SENTINEL = "chunkindex-invalid" FD_MAX_AGE = 4 * 60 # 4 minutes diff --git a/src/borg/repository.py b/src/borg/repository.py index c5b7df3548..eab4c0c037 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -953,10 +953,10 @@ def __init__( self.exclusive = exclusive self._pack_writer = None self._chunks = None # ChunkIndex; loaded lazily on first access to .chunks - # corrupt-header handling for the lazy .chunks rebuild, set by ArchiveChecker.check() (see - # PackReader.iter_headers): a validate callable makes the rebuild resync past a corrupt - # object header, drop_corrupt_tail - only set when repairing - makes it index the pack up - # to that header and drop the rest. Without either, such a header aborts the rebuild. + # corrupt-header handling for the lazy .chunks rebuild (see PackReader.iter_headers): a + # validate callable makes the rebuild resync past a corrupt object header, drop_corrupt_tail + # makes it index the pack up to that header and drop the rest. Without either, such a header + # aborts the rebuild. TODO(#10378): nothing sets them, remove both. self.chunkindex_validate = None self.chunkindex_drop_corrupt_tail = False # pack_id -> PackReader holding the whole pack; get_many loads into it, get() reuses it @@ -1145,20 +1145,18 @@ def chunks(self): @chunks.setter def chunks(self, value): - # The index is normally built lazily; this setter exists for the few callers - # that must install a specific index: wiping the cache, restoring an index - # captured before close(), or compact sharing the index it built itself (it - # needs the usage flags) so the repository does not build a second one. To - # drop a stale index so it rebuilds, do not assign None here -- call + # The index is normally built lazily; this setter installs a specific index: wiping the + # cache, restoring an index captured before close(), or an index that compact (it needs the + # usage flags) or check built itself, so the repository does not build a second one. + # To drop a stale index so it rebuilds, do not assign None here -- call # invalidate_chunk_index() instead. self._chunks = value def invalidate_chunk_index(self): - """Drop the in-memory chunk index so close() will not persist a stale copy. + """Drop the in-memory chunk index, so close() does not persist it and its memory is freed. - Called when the on-disk chunk index is deleted; the next access to - .chunks rebuilds the index from actual repository contents. PackWriter - reads the index through this Repository, so it follows automatically. + The next access to .chunks builds the index again. PackWriter reads the index through this + Repository, so it uses the new one. """ self._chunks = None @@ -1341,8 +1339,8 @@ def store_list(namespace): # the index is checked first and in full, on partial checks too: it is small, and index errors # stop the pack check below. index_infos = store_list("index") - # an interrupted fragment deletion leaves the invalid marker set; the index is rebuilt on next - # use, so warn rather than fail. + # with the invalid marker set, the index/ fragments may be missing entries or point at deleted + # packs (see write_chunkindex_invalid). The next use rebuilds the index from the packs, so warn. from .cache import chunkindex_is_invalid, build_chunkindex_from_repo index_invalid = chunkindex_is_invalid(self) @@ -1685,13 +1683,21 @@ def put(self, id, data): def delete(self, id, *, validate, update_index=True): """Delete a single repo object by rewriting its pack without it (via compact_pack). - With update_index=True the full chunk index is written back so the next borg process sees the - deletion; callers that rebuild the index themselves (check --repair) pass update_index=False to - skip the per-object index rewrite. + The rewrite deletes the old pack, so the index/ fragments point the pack's other objects at a + deleted pack until the index is stored again. The invalid marker (see write_chunkindex_invalid) is + written via compact_pack's before_old_pack_delete, just before the old pack is deleted. + Raises PermissionDenied before any store change unless the repo permissions grant write and delete + on packs/ and index/ (see assert_writable). + + update_index: True: store the full chunk index and delete the invalid marker. False: update the + in-memory index only; the marker stays until the index is stored and the marker deleted. validate: passed to compact_pack. """ + from .cache import write_chunkindex_to_repo, write_chunkindex_invalid, delete_chunkindex_invalid + self._lock_refresh() + self.assert_writable() entry = self.chunks.get(id) if entry is None: raise self.ObjectNotFound(id, str(self._location)) @@ -1699,15 +1705,22 @@ def delete(self, id, *, validate, update_index=True): # keep every object the chunk index lists for this pack, except the one being deleted. keep_ids = {cid for cid, e in self.chunks.iteritems() if e.pack_id == pack_id} keep_ids.discard(id) - self.compact_pack(pack_id, keep_ids=keep_ids, drop_ids={id}, validate=validate) + self.compact_pack( + pack_id, + keep_ids=keep_ids, + drop_ids={id}, + validate=validate, + before_old_pack_delete=lambda: write_chunkindex_invalid(self), + ) if update_index: # close() only persists new entries incrementally, so write the full index here to record # the removal for the next borg process. - from .cache import write_chunkindex_to_repo - write_chunkindex_to_repo(self, self.chunks, incremental=False, force_write=True, delete_other=True) + delete_chunkindex_invalid(self) - def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, validate, chunks=None): + def compact_pack( + self, pack_id, *, keep_ids: set, drop_ids: set, validate, chunks=None, before_old_pack_delete=None + ): """Rewrite pack , keeping and dropping , then delete the old pack. keep_ids: chunk ids in this pack to copy into the new pack. @@ -1715,6 +1728,8 @@ def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, validate, chunk validate: passed to superseded_gap_ranges, whose ranges are dropped. chunks: the ChunkIndex to look up the objects' pack locations in and to apply the index updates to. Must be the index keep_ids and drop_ids were derived from. Default: self.chunks. + before_old_pack_delete: callable without arguments, called once just before the old pack is deleted. + Not called when no bytes are dropped, since the old pack then stays. Together, keep_ids and drop_ids must cover every object the chunk index lists for this pack; an unlisted indexed object would keep its bytes in the new pack but its index entry would go @@ -1730,8 +1745,7 @@ def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, validate, chunk unchanged pack_id if nothing was dropped; dropped_bytes is the on-disk bytes this rewrite freed (unused indexed objects plus superseded duplicates), for --stats accounting. - Updates the in-memory chunk index only; the caller holds the exclusive lock and writes the - index back to the store afterwards. + Updates the in-memory chunk index only; requires the exclusive lock. """ self._lock_refresh() if chunks is None: @@ -1789,6 +1803,11 @@ def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set, validate, chunk else: new_pack_id = None # every byte was dropped: no replacement pack + # the new pack is not in the index/ fragments yet, so they still match the store; deleting the old + # pack makes them point at a deleted pack. + if before_old_pack_delete is not None and new_pack_id != pack_id: + before_old_pack_delete() + for drop_id in drop_ids: # remove dropped objects from the index del chunks[drop_id] diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 8706c12059..f0d53d9ac3 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -1,7 +1,9 @@ +import gc from pathlib import Path import re import shutil import struct +import weakref from unittest.mock import patch import pytest @@ -9,11 +11,19 @@ from ...crypto.key import store_hash, STORE_HASH_NAME from ... import archive as archive_module from ...archive import Archive, ArchiveChecker, ChunkBuffer -from ...cache import Cache, delete_chunkindex_from_repo +from ...cache import ( + Cache, + chunkindex_is_invalid, + delete_chunkindex_from_repo, + list_chunkindex_hashes, + read_chunkindex_from_repo, + write_chunkindex_invalid, +) from ...constants import * # NOQA from ...helpers import bin_to_hex, msgpack, CommandError, CorruptPack, Error, IntegrityError, sig_int from ...helpers import BackupDamagedChunksError from ...helpers.passphrase import PassphraseWrong +from ...hashindex import ChunkIndex from ...item import Item from ...manifest import Archives, Manifest from ...repoobj import RepoObj @@ -26,6 +36,7 @@ create_src_archive, create_regular_file, open_archive, + open_repository, generate_archiver_tests, read_chunk, write_wrong_content_chunk, @@ -443,12 +454,17 @@ def test_missing_file_chunk_refs_truncated(archivers, request): assert f"only the first {cap} files are listed" in output # the remaining referencing files are truncated -def test_missing_archive_item_chunk(archivers, request): - archiver = request.getfixturevalue(archivers) +def delete_first_item_chunk(archiver): + """Set up two archives and delete the first item metadata chunk of archive1.""" check_cmd_setup(archiver) archive, repository = open_archive(archiver.repository_path, "archive1") with repository: repository.delete(archive.item_ids[0], validate=None) + + +def test_missing_archive_item_chunk(archivers, request): + archiver = request.getfixturevalue(archivers) + delete_first_item_chunk(archiver) cmd(archiver, "check", exit_code=1) cmd(archiver, "check", "--repair", exit_code=0) cmd(archiver, "check", exit_code=0) @@ -465,6 +481,191 @@ def test_missing_archive_metadata(archivers, request): cmd(archiver, "check", exit_code=0) +# checker_builds: per index build in ArchiveChecker, whether repository.chunks was loaded at that time. +# A full check without --repair uses the index the repository check loaded, --repair also builds in finish(). +@pytest.mark.parametrize( + "args, exit_code, checker_builds", + [(["--archives-only"], 1, [False]), ([], 1, []), (["--repair"], 0, [False, False])], + ids=["archives-only", "full", "repair"], +) +def test_check_holds_a_single_chunk_index(archiver, monkeypatch, args, exit_code, checker_builds): + """check has at most one chunk index in memory: the repository and the checker use the same index.""" + # local-only: this patches in-process archive and repository internals. + # with an item metadata chunk missing, --repair stores a new item metadata stream. + delete_first_item_chunk(archiver) + + loaded_at_build = [] + built = [] # weak references to the indexes the checker built + alive_at_build = [] # per index build in ArchiveChecker: whether an index it built before is still alive + real_build = archive_module.build_chunkindex_from_repo + + def build_chunkindex_from_repo(repository, **kwargs): + loaded_at_build.append(repository.is_chunk_index_loaded) + gc.collect() # PyPy frees objects only on collection + alive_at_build.append(any(ref() is not None for ref in built)) + chunks = real_build(repository, **kwargs) + built.append(weakref.ref(chunks)) + return chunks + + repository_builds = 0 # index builds by the Repository.chunks property + real_chunks = Repository.chunks + + def chunks(self): + nonlocal repository_builds + if not self.is_chunk_index_loaded: + repository_builds += 1 + return real_chunks.fget(self) + + same_index = [] # per rebuild_archives call: whether repository.chunks is the checker's index + real_rebuild_archives = ArchiveChecker.rebuild_archives + + def rebuild_archives(self, **kwargs): + same_index.append(self.repository.chunks is self.chunks) + return real_rebuild_archives(self, **kwargs) + + monkeypatch.setattr(archive_module, "build_chunkindex_from_repo", build_chunkindex_from_repo) + monkeypatch.setattr(Repository, "chunks", property(chunks, real_chunks.fset)) + monkeypatch.setattr(ArchiveChecker, "rebuild_archives", rebuild_archives) + cmd(archiver, "check", *args, exit_code=exit_code) + + assert loaded_at_build == checker_builds + assert alive_at_build == [False] * len(checker_builds) + assert same_index == [True] + assert repository_builds == 0 + if "--repair" in args: + cmd(archiver, "check", exit_code=0) + + +@pytest.mark.parametrize("index", ["index", "no-index", "marker"]) +def test_check_without_repair_stores_no_chunk_index(archivers, request, index): + """check without --repair does not store a chunk index. + + The archive has an item metadata chunk missing: the checker re-chunks its item metadata stream into + chunks the repository does not have. With index/ fragments, the check leaves them as they are. Without + them, the checker builds the index from the packs and does not store it. With the invalid marker set, + that build deletes the fragments and the marker, and the check does not store its index either. + """ + archiver = request.getfixturevalue(archivers) + delete_first_item_chunk(archiver) + with open_repository(archiver) as repository: + chunk_ids_before = {chunk_id for chunk_id, _ in repository.chunks.iteritems()} + if index == "no-index": + delete_chunkindex_from_repo(repository) + index_before = list_chunkindex_hashes(repository) + assert bool(index_before) is (index != "no-index") + if index == "marker": + write_chunkindex_invalid(repository) + + cmd(archiver, "check", "--archives-only", exit_code=1) + cmd(archiver, "check", exit_code=1) + + with open_repository(archiver) as repository: + # check the stored state before .chunks rebuilds the index and close() stores it. + assert list_chunkindex_hashes(repository) == (index_before if index == "index" else []) + assert not chunkindex_is_invalid(repository) + assert {chunk_id for chunk_id, _ in repository.chunks.iteritems()} == chunk_ids_before + + +@pytest.mark.parametrize("repair", [False, True], ids=["check", "repair"]) +def test_check_with_buffered_chunks(archiver, repair): + """ArchiveChecker.check() stores the chunks the pack writer still buffers before it uses the index.""" + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + data = b"buffered chunk" + chunk_id = archive.key.id_hash(data) + repository.put(chunk_id, archive.repo_objs.format(chunk_id, {}, data, ro_type=ROBJ_FILE_STREAM)) + assert repository.chunks[chunk_id].flags & ChunkIndex.F_PENDING + ArchiveChecker().check(repository, verify_data=True, repair=repair, sort_by="ts", format="{archive}") + with open_repository(archiver) as repository: + assert repository.get(chunk_id) + + +def test_check_repair_verify_data_aborted_marks_the_index_invalid(archiver, monkeypatch): + """A --repair --verify-data check that stops after a delete leaves the chunk index marked invalid. + + delete() rewrites the pack of the defect chunk, so the index/ fragments point its other chunks at a + pack that is gone. The marker makes the next use rebuild the index from the packs. + """ + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + for item in archive.iter_items(): + if item.path.endswith(src_file): + defect_id = item.chunks[-1].id + break + corrupt_chunk_on_disk(repository, defect_id) + chunk_ids = {chunk_id for chunk_id, _ in repository.chunks.iteritems()} - {defect_id} + + def rebuild_archives(self, **kwargs): + raise Error("stopped before finish()") + + with monkeypatch.context() as m: + m.setattr(ArchiveChecker, "rebuild_archives", rebuild_archives) + with open_repository(archiver) as repository: + with pytest.raises(Error, match="stopped before finish"): + ArchiveChecker().check(repository, verify_data=True, repair=True, sort_by="ts", format="{archive}") + + with open_repository(archiver) as repository: + # no .chunks access or get() here: rebuilding the index would clear the marker. + assert chunkindex_is_invalid(repository) + # the index/ fragments are still there and point chunks at the pack delete() removed. + packs = {info.name for info in repository.store_list("packs")} + stale = set() + for hash in list_chunkindex_hashes(repository): + fragment = read_chunkindex_from_repo(repository, hash) + stale |= {chunk_id for chunk_id, entry in fragment.items() if bin_to_hex(entry.pack_id) not in packs} + assert stale & chunk_ids + cmd(archiver, "check", "--repair", exit_code=0) + with open_repository(archiver) as repository: + assert not chunkindex_is_invalid(repository) + # the stored index finds every other chunk. + for chunk_id in chunk_ids: + repository.get(chunk_id) + + +def test_check_repair_stopped_in_the_index_rebuild_marks_the_index_invalid(archiver, monkeypatch): + """A --repair check that stops in the index rebuild of finish() leaves the chunk index marked invalid. + + The repair stored a new item metadata stream and new archive metadata, which the index/ fragments do not + have. The marker makes the next use rebuild the index from the packs, which have them. + """ + delete_first_item_chunk(archiver) + real_build = archive_module.build_chunkindex_from_repo + + def build_chunkindex_from_repo(repository, **kwargs): + if kwargs.get("write_immediately"): # the rebuild in finish() + raise Error("stopped in the index rebuild") + return real_build(repository, **kwargs) + + with monkeypatch.context() as m: + m.setattr(archive_module, "build_chunkindex_from_repo", build_chunkindex_from_repo) + with open_repository(archiver) as repository: + with pytest.raises(Error, match="stopped in the index rebuild"): + ArchiveChecker().check(repository, repair=True, sort_by="ts", format="{archive}") + + with open_repository(archiver) as repository: + assert chunkindex_is_invalid(repository) + assert list_chunkindex_hashes(repository) + cmd(archiver, "check", exit_code=0) + with open_repository(archiver) as repository: + assert not chunkindex_is_invalid(repository) + + +def test_check_repair_clears_the_invalid_marker(archivers, request): + """check --repair clears the invalid marker when it stores the index, also without index/ fragments before.""" + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + with open_repository(archiver) as repository: + delete_chunkindex_from_repo(repository) + write_chunkindex_invalid(repository) + cmd(archiver, "check", "--repair", exit_code=0) + with open_repository(archiver) as repository: + assert not chunkindex_is_invalid(repository) + assert list_chunkindex_hashes(repository) + cmd(archiver, "check", exit_code=0) + + def test_check_format(archivers, request): archiver = request.getfixturevalue(archivers) check_cmd_setup(archiver) diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 80525924c4..c075a362c7 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -11,7 +11,7 @@ from ..crypto.key import store_hash from .. import repository as repository_module -from ..cache import write_chunkindex_invalid +from ..cache import chunkindex_is_invalid, delete_chunkindex_from_repo, write_chunkindex_invalid from ..compress import CNONE from ..constants import MAX_CLOCK_SKEW, ROBJ_FILE_STREAM from ..crypto.key import CHPOKey, ChecksumKey @@ -443,6 +443,90 @@ def test_delete_with_stale_earlier_object_in_pack(repo_fixtures, request): assert pdchunk(repository.get(H(0))) == b"ccc" # H(0) still served from its new pack +@pytest.mark.parametrize("stored", [True, False], ids=["stored-index", "no-stored-index"]) +@pytest.mark.parametrize("update_index", [True, False]) +def test_delete_marks_the_chunk_index_invalid(repo_fixtures, request, update_index, stored): + """delete() marks the chunk index invalid before it deletes the old pack; update_index=True clears the marker. + + Without stored fragments, the full index write deletes none and leaves the marker; delete() deletes it. + """ + repository = get_repository_from_fixture(repo_fixtures, request) + build_one_pack(repository, [(H(0), fchunk(b"aaa", chunk_id=H(0))), (H(1), fchunk(b"bbb", chunk_id=H(1)))]) + with reopen(repository) as repository: + if not stored: + delete_chunkindex_from_repo(repository) + assert bool(list(repository.store_list("index"))) is stored + repository.delete(H(1), validate=None, update_index=update_index) + assert chunkindex_is_invalid(repository) is not update_index + with reopen(repository) as repository: + assert pdchunk(repository.get(H(0))) == b"aaa" + with pytest.raises(Repository.ObjectNotFound): + repository.get(H(1)) + + +def test_delete_missing_object_leaves_the_chunk_index_valid(repo_fixtures, request): + """delete() of an object the index does not have does not mark the chunk index invalid.""" + with get_repository_from_fixture(repo_fixtures, request) as repository: + assert H(0) not in repository.chunks # load the index before delete(): loading it deletes the marker + with pytest.raises(Repository.ObjectNotFound): + repository.delete(H(0), validate=None) + assert not chunkindex_is_invalid(repository) + + +@pytest.mark.parametrize("fail", ["overlap", "past-end", "no-delete"]) +def test_delete_refused_leaves_the_chunk_index_valid(repo_fixtures, request, monkeypatch, fail): + """A delete() refused before changing the store leaves no marker and the fragments and packs unchanged.""" + repository = get_repository_from_fixture(repo_fixtures, request) + build_one_pack(repository, [(H(0), fchunk(b"aaa", chunk_id=H(0))), (H(1), fchunk(b"bbb", chunk_id=H(1)))]) + if fail == "no-delete": + monkeypatch.setenv("BORG_REPO_PERMISSIONS", "no-delete") + with reopen(repository) as repository: + pack_key = "packs/" + bin_to_hex(repository.chunks[H(0)].pack_id) + fragments = sorted(info.name for info in repository.store_list("index")) + packs = sorted(info.name for info in repository.store_list("packs")) + if fail == "overlap": # check_pack_objects: H(1) overlaps H(0) + entry = repository.chunks[H(1)] + repository.chunks[H(1)] = entry._replace(obj_offset=repository.chunks[H(0)].obj_offset) + elif fail == "past-end": # check_pack_objects: H(1) ends past the truncated pack + repository.store.store(pack_key, repository.store_load(pack_key)[:-1]) + expected = Repository.PermissionDenied if fail == "no-delete" else IntegrityError + with pytest.raises(expected): + repository.delete(H(0), validate=None) + assert not chunkindex_is_invalid(repository) + assert sorted(info.name for info in repository.store_list("index")) == fragments + assert sorted(info.name for info in repository.store_list("packs")) == packs + + +def test_delete_stopped_in_the_pack_rewrite_leaves_the_chunk_index_invalid(repo_fixtures, request, monkeypatch): + """A delete() that stops after deleting the old pack leaves the marker; the next use rebuilds the index.""" + with get_repository_from_fixture(repo_fixtures, request) as repository: + repository._pack_writer.max_count = 2 # H(0) and H(1) share a pack + repository.put(H(0), fchunk(b"aaa", chunk_id=H(0))) + repository.put(H(1), fchunk(b"bbb", chunk_id=H(1))) + repository.flush() + old_pack_id = None + with pytest.raises(OSError, match="stopped"): + with reopen(repository) as repository: + old_pack_id = repository.chunks[H(0)].pack_id + real_store_delete = repository.store_delete + + def store_delete(name, *args, **kwargs): + if name.startswith("packs/"): + real_store_delete(name, *args, **kwargs) # the old pack is gone, the index not stored yet + raise OSError("stopped") + return real_store_delete(name, *args, **kwargs) + + monkeypatch.setattr(repository, "store_delete", store_delete) + repository.delete(H(1), validate=None) + with reopen(repository) as repository: + assert chunkindex_is_invalid(repository) + with reopen(repository) as repository: + # the index rebuilt from the packs points H(0) at the rewritten pack. + assert repository.chunks[H(0)].pack_id != old_pack_id + assert pdchunk(repository.get(H(0))) == b"aaa" + assert not chunkindex_is_invalid(repository) + + def test_multi_object_pack_roundtrip(repo_fixtures, request): # Two objects fill one pack and must both read back: the second from a non-zero offset, and # read_data=False returning only its header+meta. The test pins max_count=2 so it does not depend