From 913eb3ed8cf9bb59591281674bebf72dd7366f88 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Tue, 14 Jul 2026 16:41:01 +0530 Subject: [PATCH 1/5] cache: write a "chunk index invalidated" sentinel before deleting index fragments (#9904) An interrupted fragment deletion could leave a subset that build_chunkindex_from_repo trusts as a complete index; the sentinel now forces a rebuild from packs instead. --- src/borg/cache.py | 77 ++++++++++-- src/borg/constants.py | 5 + src/borg/repository.py | 5 + .../testsuite/archiver/compact_cmd_test.py | 4 +- src/borg/testsuite/cache_test.py | 114 +++++++++++++++--- src/borg/testsuite/repository_test.py | 2 +- 6 files changed, 173 insertions(+), 34 deletions(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index 32b3a41913..61d12e1857 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -22,7 +22,7 @@ from .constants import CACHE_README, FILES_CACHE_MODE_DISABLED, ROBJ_FILE_STREAM, TIME_DIFFERS2_NS from .constants import CHUNKINDEX_FRAGMENT_ENTRIES_MIN, CHUNKINDEX_FRAGMENT_ENTRIES_MAX -from .constants import CHUNKINDEX_SMALL_FRAGMENT_CAP, CHUNKINDEX_MERGE_ATTEMPTS +from .constants import CHUNKINDEX_SMALL_FRAGMENT_CAP, CHUNKINDEX_MERGE_ATTEMPTS, CHUNKINDEX_INVALIDATED_NAME from .hashindex import ChunkIndex, ChunkIndexEntry, ChunkIndexEntryFormat from .helpers import get_cache_dir from .helpers import chunkit @@ -559,31 +559,63 @@ def list_chunkindex_fragments(repository): count is estimated from the stored object's byte size (chunkindex_fragment_entry_size() bytes per entry), so we can classify fragments (small vs. sealed) without loading them. The estimate ignores the small fixed header, which is negligible for the fragment sizes we care about. - Returns a list of (name, approx_entries) tuples, sorted by name. + Returns (fragments, invalidated): fragments is a list of (name, approx_entries) tuples sorted by + name; invalidated is True if the index-invalidated sentinel is present. The sentinel is excluded + from fragments. store_list() bypasses the borgstore cache, so the sentinel's presence stays + correct even when the index/ namespace is cache-backed. """ entry_size = chunkindex_fragment_entry_size() fragments = [] + invalidated = False for info in repository.store_list("index"): info = ItemInfo(*info) # RPC does not give namedtuple + if info.name == CHUNKINDEX_INVALIDATED_NAME: + invalidated = True + continue fragments.append((info.name, info.size // entry_size)) fragments.sort() - return fragments + return fragments, invalidated def list_chunkindex_hashes(repository): - hashes = [name for name, _ in list_chunkindex_fragments(repository)] # already sorted by name + fragments, invalidated = list_chunkindex_fragments(repository) + hashes = [name for name, _ in fragments] # already sorted by name logger.debug(f"chunk indexes: {hashes}") - return hashes + return hashes, invalidated + + +def write_chunkindex_invalidated(repository): + """Write the index-invalidated sentinel. Call before deleting index fragments. + + If the deletion is interrupted, the sentinel remains and the index is rebuilt on next load. Only + the sentinel's presence matters; its content is empty. + """ + repository.store_store(f"index/{CHUNKINDEX_INVALIDATED_NAME}", b"") + + +def delete_chunkindex_invalidated(repository): + """Remove the index-invalidated sentinel. Call after all fragment deletions have completed.""" + try: + repository.store_delete(f"index/{CHUNKINDEX_INVALIDATED_NAME}") + except StoreObjectNotFound: + pass def delete_chunkindex_from_repo(repository): - hashes = list_chunkindex_hashes(repository) + hashes, invalidated = list_chunkindex_hashes(repository) + if hashes: + # mark invalidated before deleting the first fragment, so an interrupted deletion is detectable. + write_chunkindex_invalidated(repository) for hash in hashes: index_name = f"index/{hash}" try: repository.store_delete(index_name) except StoreObjectNotFound: pass + if hashes or invalidated: + # remove the sentinel after every fragment is gone; also clears a sentinel left behind by an + # earlier interrupted deletion. + delete_chunkindex_invalidated(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 # index we just deleted. the next .chunks access rebuilds it from actual repo contents. @@ -626,7 +658,8 @@ def write_chunkindex_to_repo( # bounded, immutable fragments over one big index. max_entries = CHUNKINDEX_FRAGMENT_ENTRIES_MAX # the fragment set present in the repo before we start writing: - stored_hashes = set(list_chunkindex_hashes(repository)) + hashes, _ = list_chunkindex_hashes(repository) + stored_hashes = set(hashes) new_hashes = set() # content hashes of the fragments that make up the index we are writing now fragments_written = 0 @@ -687,12 +720,20 @@ def write_chunkindex_to_repo( delete_these = set(stored_hashes) - new_hashes else: delete_these = set(delete_these) - new_hashes + # A delete_other rewrite may drop entries, so leftover fragments after a mid-delete crash must + # not be merged back; guard that deletion with the sentinel. Otherwise the deleted fragments' + # entries are already contained in the fragments we just wrote, so leftovers are harmless. + guard = delete_other and bool(delete_these) + if guard: + write_chunkindex_invalidated(repository) for hash in delete_these: index_name = f"index/{hash}" try: repository.store_delete(index_name) except StoreObjectNotFound: pass + if guard: + delete_chunkindex_invalidated(repository) if delete_these: logger.debug(f"chunk indexes deleted: {delete_these}") return new_hashes @@ -729,11 +770,11 @@ def repack_chunkindex(repository): sum to >= MIN) or when too many small fragments have piled up (more than CHUNKINDEX_SMALL_FRAGMENT_CAP), so we don't rewrite a slowly growing fragment on every backup. """ - small = [ - (name, approx) - for name, approx in list_chunkindex_fragments(repository) - if approx < CHUNKINDEX_FRAGMENT_ENTRIES_MIN - ] + fragments, invalidated = list_chunkindex_fragments(repository) + if invalidated: + # the index is invalidated; it will be rebuilt on next load, so there is nothing to consolidate. + return + small = [(name, approx) for name, approx in fragments if approx < CHUNKINDEX_FRAGMENT_ENTRIES_MIN] if len(small) < 2: return # nothing to gain from merging zero or one fragment small_total = sum(approx for _, approx in small) @@ -778,7 +819,17 @@ def build_chunkindex_from_repo( # the fresh listing contains the replacement fragment. if we cannot get a complete, consistent # set (e.g. a persistently unreadable fragment), fall through to the slow rebuild instead. for _ in range(CHUNKINDEX_MERGE_ATTEMPTS): - hashes = list_chunkindex_hashes(repository) + hashes, invalidated = list_chunkindex_hashes(repository) + if invalidated: + # the index is invalidated: surviving fragments may be incomplete or stale. Finish the + # interrupted deletion (best-effort; a read-only client rebuilds in memory only), then + # rebuild from packs. + logger.warning("chunk index was invalidated by an interrupted operation, rebuilding it.") + try: + delete_chunkindex_from_repo(repository) + except Exception as err: + logger.debug(f"could not remove invalidated chunk index fragments: {err!r}") + break if not hashes: # no chunk index fragments available break chunks = ChunkIndex() # we'll merge all fragments into this diff --git a/src/borg/constants.py b/src/borg/constants.py index 27bfda0eb1..018b4510da 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -114,6 +114,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 +# Sentinel in the index/ namespace marking the chunk index as invalidated. Written before deleting +# fragments and removed afterwards; while present, the index is rebuilt from packs. The all-zeros +# name is a valid but unreachable sha256 (nothing hashes to it), so older borg reads it as an +# unreadable fragment and falls back to the rebuild. +CHUNKINDEX_INVALIDATED_NAME = "0" * 64 FD_MAX_AGE = 4 * 60 # 4 minutes diff --git a/src/borg/repository.py b/src/borg/repository.py index 580507519f..2c564fd1cb 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -707,6 +707,11 @@ def store_list(namespace): # stop and report that rather than continue. matters for partial checks too, whose runs can be # days apart (e.g. a weekend cron job). index_infos = store_list("index") + # the index-invalidated sentinel is not a real fragment and its content does not match its + # name; skip verification and warn rather than report it as corruption. + if any(info.name == CHUNKINDEX_INVALIDATED_NAME for info in index_infos): + logger.warning("chunk index was invalidated by an interrupted operation; it will be rebuilt on next use.") + index_infos = [info for info in index_infos if info.name != CHUNKINDEX_INVALIDATED_NAME] index_pi = ProgressIndicatorPercent(total=len(index_infos), msg="Checking index %3.0f%%", msgid="check.index") for info in index_infos: self._lock_refresh() diff --git a/src/borg/testsuite/archiver/compact_cmd_test.py b/src/borg/testsuite/archiver/compact_cmd_test.py index 88e2bf5620..c2f7d82ec1 100644 --- a/src/borg/testsuite/archiver/compact_cmd_test.py +++ b/src/borg/testsuite/archiver/compact_cmd_test.py @@ -139,7 +139,7 @@ def interrupt(): # The objects were deleted, so no readable chunk index may still list them. repository = open_repository(archiver) with repository: - assert list_chunkindex_hashes(repository) == [] + assert list_chunkindex_hashes(repository)[0] == [] # A later backup of identical content must re-upload the deleted chunks, ... cmd(archiver, "create", "archive2", "input") @@ -193,7 +193,7 @@ def store_delete_then_interrupt(name, **kwargs): # every persisted index entry points at a pack that still exists repository = open_repository(archiver) with repository: - assert list_chunkindex_hashes(repository) != [] + assert list_chunkindex_hashes(repository)[0] != [] pack_names_after = {info.name for info in repository.store_list("packs")} assert 0 < len(pack_names_after) < len(pack_names_before) # some packs deleted, some left for id, entry in repository.chunks.iteritems(): diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index 2a109a259f..816ad6f2c9 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -18,8 +18,10 @@ list_chunkindex_hashes, read_chunkindex_from_repo, repack_chunkindex, + write_chunkindex_invalidated, write_chunkindex_to_repo, ) +from ..constants import CHUNKINDEX_INVALIDATED_NAME from ..hashindex import ChunkIndex, ChunkIndexEntry from ..crypto.key import AESOCBKey from ..helpers import safe_ns @@ -161,7 +163,7 @@ def test_build_chunkindex_retries_on_vanished_fragment(tmp_path): ci = ChunkIndex() ci[h] = ChunkIndexEntry(ChunkIndex.F_NEW, 0, h, 0, 4) write_chunkindex_to_repo(repository, ci, incremental=False, force_write=True) - fragment_names = list_chunkindex_hashes(repository) + fragment_names = list_chunkindex_hashes(repository)[0] original_store_load = repository.store_load raced = False @@ -222,7 +224,7 @@ def test_chunkindex_cache_not_consolidated_on_access(tmp_path): ci = ChunkIndex() ci[h] = ChunkIndexEntry(ChunkIndex.F_NEW, 0, h, 0, 4) write_chunkindex_to_repo(repository, ci, incremental=False, force_write=True) - before = len(list_chunkindex_hashes(repository)) + before = len(list_chunkindex_hashes(repository)[0]) assert before > 1 cache = ChunksMixin() @@ -230,7 +232,7 @@ def test_chunkindex_cache_not_consolidated_on_access(tmp_path): index = cache.chunks # binds the repository index; must NOT collapse the fragments # fragments are left intact (no consolidation side effect) ... - assert len(list_chunkindex_hashes(repository)) == before + assert len(list_chunkindex_hashes(repository)[0]) == before # ... and the in-memory index still resolves every seeded chunk assert H(1) in index and H(2) in index @@ -266,7 +268,7 @@ def test_write_chunkindex_splits_full_write(tmp_path, monkeypatch): write_chunkindex_to_repo( repository, _make_chunkindex(keys), incremental=False, force_write=True, delete_other=True ) - frags = list_chunkindex_fragments(repository) + frags = list_chunkindex_fragments(repository)[0] # 7000 entries split by MAX=3000 -> 3 fragments (3000 + 3000 + 1000) counts = sorted(len(read_chunkindex_from_repo(repository, name)) for name, _ in frags) assert counts == [1000, 3000, 3000] @@ -284,9 +286,9 @@ def test_repack_defers_when_below_min_and_few_fragments(tmp_path, monkeypatch): delete_chunkindex_from_repo(repository) for j in range(3): # 3 fragments * 100 = 300 entries < MIN, count 3 <= cap 5 _seed_fragment(repository, j * 100, 100) - before = {name for name, _ in list_chunkindex_fragments(repository)} + before = {name for name, _ in list_chunkindex_fragments(repository)[0]} repack_chunkindex(repository) - after = {name for name, _ in list_chunkindex_fragments(repository)} + after = {name for name, _ in list_chunkindex_fragments(repository)[0]} assert after == before # nothing merged @@ -302,9 +304,9 @@ def test_repack_seals_when_smalls_reach_min(tmp_path, monkeypatch): all_keys = [] for j in range(5): # 5 * 300 = 1500 >= MIN all_keys += _seed_fragment(repository, j * 300, 300) - assert len(list_chunkindex_fragments(repository)) == 5 + assert len(list_chunkindex_fragments(repository)[0]) == 5 repack_chunkindex(repository) - frags = list_chunkindex_fragments(repository) + frags = list_chunkindex_fragments(repository)[0] assert len(frags) == 1 # 1500 entries, one fragment (< MAX) merged = read_chunkindex_from_repo(repository, frags[0][0]) assert len(merged) == 1500 @@ -323,9 +325,9 @@ def test_repack_cap_forces_merge_below_min(tmp_path, monkeypatch): all_keys = [] for j in range(6): # 6 * 50 = 300 < MIN, but count 6 > cap 5 all_keys += _seed_fragment(repository, j * 50, 50) - assert len(list_chunkindex_fragments(repository)) == 6 + assert len(list_chunkindex_fragments(repository)[0]) == 6 repack_chunkindex(repository) - frags = list_chunkindex_fragments(repository) + frags = list_chunkindex_fragments(repository)[0] assert len(frags) == 1 # merged into a single sub-MIN remainder merged = read_chunkindex_from_repo(repository, frags[0][0]) assert len(merged) == 300 @@ -342,7 +344,7 @@ def test_write_chunkindex_splits_incremental_write(tmp_path, monkeypatch): keys = [_ci_key(i) for i in range(1200)] # all F_NEW -> written by the incremental path write_chunkindex_to_repo(repository, _make_chunkindex(keys), incremental=True) counts = sorted( - len(read_chunkindex_from_repo(repository, name)) for name, _ in list_chunkindex_fragments(repository) + len(read_chunkindex_from_repo(repository, name)) for name, _ in list_chunkindex_fragments(repository)[0] ) assert counts == [200, 500, 500] # 1200 split by MAX=500 @@ -364,7 +366,7 @@ def test_write_chunkindex_deterministic_fragments(tmp_path, monkeypatch): delete_chunkindex_from_repo(repository) keys = [_ci_key(i) for i in (reversed(key_ints) if reverse else key_ints)] write_chunkindex_to_repo(repository, _make_chunkindex(keys), incremental=False, force_write=True) - frags = list_chunkindex_fragments(repository) + frags = list_chunkindex_fragments(repository)[0] hashes.append({name for name, _ in frags}) # keys are big-endian ints, so sorted key order == numeric order: the batches must # hold exactly the ranges [0..499], [500..999], [1000..1199]. @@ -404,7 +406,7 @@ def test_close_consolidates_fragments_across_sessions(tmp_path, monkeypatch): repository.flush() with Repository(loc, exclusive=True) as repository: - frags = list_chunkindex_fragments(repository) + frags = list_chunkindex_fragments(repository)[0] # without repack there would be one incremental fragment per session (plus creation's empty); # repack consolidates the small ones as they accumulate, so we end up with fewer. assert len(frags) < 5 @@ -425,7 +427,7 @@ def test_repack_leaves_sealed_untouched_and_reconstructs(tmp_path, monkeypatch): with Repository(repository_location, exclusive=True, create=True) as repository: delete_chunkindex_from_repo(repository) sealed_keys = _seed_fragment(repository, 0, 2000) # >= MIN -> sealed - sealed_hashes = {name for name, _ in list_chunkindex_fragments(repository)} + sealed_hashes = {name for name, _ in list_chunkindex_fragments(repository)[0]} assert len(sealed_hashes) == 1 small_keys = [] for j in range(3): # 3 * 400 = 1200 >= MIN -> will be merged @@ -433,7 +435,7 @@ def test_repack_leaves_sealed_untouched_and_reconstructs(tmp_path, monkeypatch): repack_chunkindex(repository) - frags = {name for name, _ in list_chunkindex_fragments(repository)} + frags = {name for name, _ in list_chunkindex_fragments(repository)[0]} assert sealed_hashes <= frags # the sealed fragment was not rewritten or deleted assert len(frags) == 2 # sealed one + the merged small ones @@ -462,23 +464,99 @@ def test_repack_idempotent_deletes_sources_when_merged_exists(tmp_path, monkeypa for j in range(5): # 5 * 300 = 1500 >= MIN -> merges into one sealed fragment all_keys += _seed_fragment(repository, j * 300, 300) repack_chunkindex(repository) - sealed = {name for name, _ in list_chunkindex_fragments(repository)} + sealed = {name for name, _ in list_chunkindex_fragments(repository)[0]} assert len(sealed) == 1 # the merged, sealed fragment # simulate a repack that stored the merged fragment but crashed before deleting the sources: # the sealed fragment is present, and the same small sources exist again alongside it. for j in range(5): _seed_fragment(repository, j * 300, 300) - assert len(list_chunkindex_fragments(repository)) == 6 # sealed + 5 re-seeded small + assert len(list_chunkindex_fragments(repository)[0]) == 6 # sealed + 5 re-seeded small repack_chunkindex(repository) # merged content already exists -> upload is dedupe-skipped - frags = {name for name, _ in list_chunkindex_fragments(repository)} + frags = {name for name, _ in list_chunkindex_fragments(repository)[0]} assert frags == sealed # sources deleted; only the (unchanged) sealed fragment remains merged = read_chunkindex_from_repo(repository, next(iter(frags))) assert set(merged) == set(all_keys) +def _sentinel_present(repository): + return any(info.name == CHUNKINDEX_INVALIDATED_NAME for info in repository.store_list("index")) + + +def test_invalidated_sentinel_forces_rebuild(tmp_path): + """With the sentinel present, build discards leftover fragments instead of trusting them.""" + loc = os.fspath(tmp_path / "repository") + with Repository(loc, exclusive=True, create=True) as repository: + # a leftover fragment plus the sentinel, as an interrupted deletion would leave them. + _seed_fragment(repository, 5000, 3) + write_chunkindex_invalidated(repository) + assert _sentinel_present(repository) + + chunks = build_chunkindex_from_repo(repository) + # the leftover entry is not merged (the repo has no packs, so a correct rebuild is empty). + assert _ci_key(5000) not in chunks + # the roll-forward removed both the leftover fragments and the sentinel. + assert not _sentinel_present(repository) + assert list_chunkindex_fragments(repository)[0] == [] + + +def test_delete_chunkindex_writes_then_removes_sentinel(tmp_path): + """An interrupted delete_chunkindex_from_repo leaves the sentinel behind.""" + loc = os.fspath(tmp_path / "repository") + with Repository(loc, exclusive=True, create=True) as repository: + _seed_fragment(repository, 0, 3) + _seed_fragment(repository, 100, 3) + assert not _sentinel_present(repository) + + real_delete = repository.store.delete + calls = {"n": 0} + + def failing_delete(name, **kwargs): + # let the sentinel write happen, then fail on the first real fragment deletion. + if name.startswith("index/") and name != f"index/{CHUNKINDEX_INVALIDATED_NAME}": + calls["n"] += 1 + if calls["n"] == 1: + raise OSError("simulated crash mid-delete") + return real_delete(name, **kwargs) + + repository.store.delete = failing_delete + with pytest.raises(OSError): + delete_chunkindex_from_repo(repository) + repository.store.delete = real_delete + # the sentinel was written before the (failed) deletion and is still present. + assert _sentinel_present(repository) + + # a fresh build sees the sentinel, rolls the deletion forward, and rebuilds cleanly. + build_chunkindex_from_repo(repository) + assert not _sentinel_present(repository) + + +def test_repack_skips_when_invalidated(tmp_path, monkeypatch): + """Repack does nothing while the index is invalidated (a rebuild will supersede it).""" + monkeypatch.setattr(cache_mod, "CHUNKINDEX_FRAGMENT_ENTRIES_MIN", 1000) + monkeypatch.setattr(cache_mod, "CHUNKINDEX_SMALL_FRAGMENT_CAP", 2) + loc = os.fspath(tmp_path / "repository") + with Repository(loc, exclusive=True, create=True) as repository: + delete_chunkindex_from_repo(repository) + for j in range(4): + _seed_fragment(repository, j * 100, 100) + write_chunkindex_invalidated(repository) + before = {name for name, _ in list_chunkindex_fragments(repository)[0]} + repack_chunkindex(repository) + after = {name for name, _ in list_chunkindex_fragments(repository)[0]} + assert before == after # unchanged: repack bailed out + + +def test_read_sentinel_returns_none(tmp_path): + """Reading the sentinel as a fragment returns None (the old-borg graceful-degradation path).""" + loc = os.fspath(tmp_path / "repository") + with Repository(loc, exclusive=True, create=True) as repository: + write_chunkindex_invalidated(repository) + assert read_chunkindex_from_repo(repository, CHUNKINDEX_INVALIDATED_NAME) is None + + def test_files_cache_save_tolerates_missing_chunk(tmp_path, monkeypatch): """A files-cache entry whose chunk vanished from the index is dropped, not fatal. diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index f9e4125979..bccc557063 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -119,7 +119,7 @@ def test_chunk_index_persisted_on_close(tmp_path): # reopen and read the cached fragments straight from disk with Repository(location, exclusive=True) as repository: persisted = ChunkIndex() - for hash in list_chunkindex_hashes(repository): + for hash in list_chunkindex_hashes(repository)[0]: fragment = read_chunkindex_from_repo(repository, hash) if fragment is not None: for k, v in fragment.items(): From 3cad4fbc17452742d4a51e6976f898345b857b94 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 15 Jul 2026 01:58:36 +0530 Subject: [PATCH 2/5] cache: move chunk-index-invalid marker out of band into config/ namespace Store the marker as config/chunkindex-invalid instead of an all-zeros index/ fragment name, so it can never collide with a real fragment; detect it via a config listing. --- src/borg/cache.py | 91 +++++++++--------- src/borg/constants.py | 10 +- src/borg/repository.py | 9 +- .../testsuite/archiver/compact_cmd_test.py | 4 +- src/borg/testsuite/cache_test.py | 96 +++++++++---------- src/borg/testsuite/repository_test.py | 15 ++- 6 files changed, 116 insertions(+), 109 deletions(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index 61d12e1857..c73f636aaf 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -22,7 +22,7 @@ from .constants import CACHE_README, FILES_CACHE_MODE_DISABLED, ROBJ_FILE_STREAM, TIME_DIFFERS2_NS from .constants import CHUNKINDEX_FRAGMENT_ENTRIES_MIN, CHUNKINDEX_FRAGMENT_ENTRIES_MAX -from .constants import CHUNKINDEX_SMALL_FRAGMENT_CAP, CHUNKINDEX_MERGE_ATTEMPTS, CHUNKINDEX_INVALIDATED_NAME +from .constants import CHUNKINDEX_SMALL_FRAGMENT_CAP, CHUNKINDEX_MERGE_ATTEMPTS, CHUNKINDEX_INVALID_SENTINEL from .hashindex import ChunkIndex, ChunkIndexEntry, ChunkIndexEntryFormat from .helpers import get_cache_dir from .helpers import chunkit @@ -551,6 +551,7 @@ def chunkindex_fragment_entry_size(): return key_size + value_size +# TODO: refactor the chunk-index functions below into a dedicated index-management class. def list_chunkindex_fragments(repository): """List the index/ fragments, returning each fragment's (name, approximate entry count). @@ -559,63 +560,63 @@ def list_chunkindex_fragments(repository): count is estimated from the stored object's byte size (chunkindex_fragment_entry_size() bytes per entry), so we can classify fragments (small vs. sealed) without loading them. The estimate ignores the small fixed header, which is negligible for the fragment sizes we care about. - Returns (fragments, invalidated): fragments is a list of (name, approx_entries) tuples sorted by - name; invalidated is True if the index-invalidated sentinel is present. The sentinel is excluded - from fragments. store_list() bypasses the borgstore cache, so the sentinel's presence stays - correct even when the index/ namespace is cache-backed. + Returns a list of (name, approx_entries) tuples, sorted by name. """ entry_size = chunkindex_fragment_entry_size() fragments = [] - invalidated = False for info in repository.store_list("index"): info = ItemInfo(*info) # RPC does not give namedtuple - if info.name == CHUNKINDEX_INVALIDATED_NAME: - invalidated = True - continue fragments.append((info.name, info.size // entry_size)) fragments.sort() - return fragments, invalidated + return fragments def list_chunkindex_hashes(repository): - fragments, invalidated = list_chunkindex_fragments(repository) - hashes = [name for name, _ in fragments] # already sorted by name + hashes = [name for name, _ in list_chunkindex_fragments(repository)] # already sorted by name logger.debug(f"chunk indexes: {hashes}") - return hashes, invalidated + return hashes + + +def chunkindex_is_invalid(repository): + """Return whether the chunk-index-invalid marker is present. + + store_list() bypasses the borgstore cache, so this stays correct even for a cache-backed namespace. + """ + return any(ItemInfo(*info).name == CHUNKINDEX_INVALID_SENTINEL for info in repository.store_list("config")) -def write_chunkindex_invalidated(repository): - """Write the index-invalidated sentinel. Call before deleting index fragments. +def write_chunkindex_invalid(repository): + """Mark the chunk index as invalid. Call before deleting index fragments. - If the deletion is interrupted, the sentinel remains and the index is rebuilt on next load. Only - the sentinel's presence matters; its content is empty. + If the deletion is interrupted, the marker remains and the index is rebuilt on next load. """ - repository.store_store(f"index/{CHUNKINDEX_INVALIDATED_NAME}", b"") + repository.store_store(f"config/{CHUNKINDEX_INVALID_SENTINEL}", b"") -def delete_chunkindex_invalidated(repository): - """Remove the index-invalidated sentinel. Call after all fragment deletions have completed.""" +def delete_chunkindex_invalid(repository): + """Clear the chunk-index-invalid marker. Call after all fragment deletions have completed.""" try: - repository.store_delete(f"index/{CHUNKINDEX_INVALIDATED_NAME}") + repository.store_delete(f"config/{CHUNKINDEX_INVALID_SENTINEL}") except StoreObjectNotFound: pass def delete_chunkindex_from_repo(repository): - hashes, invalidated = list_chunkindex_hashes(repository) + hashes = list_chunkindex_hashes(repository) + invalid = chunkindex_is_invalid(repository) if hashes: - # mark invalidated before deleting the first fragment, so an interrupted deletion is detectable. - write_chunkindex_invalidated(repository) + # mark invalid before deleting the first fragment, so an interrupted deletion is detectable. + write_chunkindex_invalid(repository) for hash in hashes: index_name = f"index/{hash}" try: repository.store_delete(index_name) except StoreObjectNotFound: pass - if hashes or invalidated: - # remove the sentinel after every fragment is gone; also clears a sentinel left behind by an + if hashes or invalid: + # clear the marker after every fragment is gone; also clears a marker left behind by an # earlier interrupted deletion. - delete_chunkindex_invalidated(repository) + 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 # index we just deleted. the next .chunks access rebuilds it from actual repo contents. @@ -658,8 +659,7 @@ def write_chunkindex_to_repo( # bounded, immutable fragments over one big index. max_entries = CHUNKINDEX_FRAGMENT_ENTRIES_MAX # the fragment set present in the repo before we start writing: - hashes, _ = list_chunkindex_hashes(repository) - stored_hashes = set(hashes) + stored_hashes = set(list_chunkindex_hashes(repository)) new_hashes = set() # content hashes of the fragments that make up the index we are writing now fragments_written = 0 @@ -721,11 +721,12 @@ def write_chunkindex_to_repo( else: delete_these = set(delete_these) - new_hashes # A delete_other rewrite may drop entries, so leftover fragments after a mid-delete crash must - # not be merged back; guard that deletion with the sentinel. Otherwise the deleted fragments' - # entries are already contained in the fragments we just wrote, so leftovers are harmless. + # not be merged back; guard that deletion with the invalid marker. Otherwise the deleted + # fragments' entries are already contained in the fragments we just wrote, so leftovers are + # harmless. guard = delete_other and bool(delete_these) if guard: - write_chunkindex_invalidated(repository) + write_chunkindex_invalid(repository) for hash in delete_these: index_name = f"index/{hash}" try: @@ -733,7 +734,7 @@ def write_chunkindex_to_repo( except StoreObjectNotFound: pass if guard: - delete_chunkindex_invalidated(repository) + delete_chunkindex_invalid(repository) if delete_these: logger.debug(f"chunk indexes deleted: {delete_these}") return new_hashes @@ -770,11 +771,14 @@ def repack_chunkindex(repository): sum to >= MIN) or when too many small fragments have piled up (more than CHUNKINDEX_SMALL_FRAGMENT_CAP), so we don't rewrite a slowly growing fragment on every backup. """ - fragments, invalidated = list_chunkindex_fragments(repository) - if invalidated: - # the index is invalidated; it will be rebuilt on next load, so there is nothing to consolidate. + if chunkindex_is_invalid(repository): + # the index is invalid; it will be rebuilt on next load, so there is nothing to consolidate. return - small = [(name, approx) for name, approx in fragments if approx < CHUNKINDEX_FRAGMENT_ENTRIES_MIN] + small = [ + (name, approx) + for name, approx in list_chunkindex_fragments(repository) + if approx < CHUNKINDEX_FRAGMENT_ENTRIES_MIN + ] if len(small) < 2: return # nothing to gain from merging zero or one fragment small_total = sum(approx for _, approx in small) @@ -819,17 +823,16 @@ def build_chunkindex_from_repo( # the fresh listing contains the replacement fragment. if we cannot get a complete, consistent # set (e.g. a persistently unreadable fragment), fall through to the slow rebuild instead. for _ in range(CHUNKINDEX_MERGE_ATTEMPTS): - hashes, invalidated = list_chunkindex_hashes(repository) - if invalidated: - # the index is invalidated: surviving fragments may be incomplete or stale. Finish the - # interrupted deletion (best-effort; a read-only client rebuilds in memory only), then - # rebuild from packs. - logger.warning("chunk index was invalidated by an interrupted operation, rebuilding it.") + if chunkindex_is_invalid(repository): + # leftover fragments may be incomplete or stale. Finish the interrupted deletion + # (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: delete_chunkindex_from_repo(repository) except Exception as err: - logger.debug(f"could not remove invalidated chunk index fragments: {err!r}") + logger.debug(f"could not remove invalid chunk index fragments: {err!r}") break + hashes = list_chunkindex_hashes(repository) if not hashes: # no chunk index fragments available break chunks = ChunkIndex() # we'll merge all fragments into this diff --git a/src/borg/constants.py b/src/borg/constants.py index 018b4510da..effda55c37 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -114,11 +114,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 -# Sentinel in the index/ namespace marking the chunk index as invalidated. Written before deleting -# fragments and removed afterwards; while present, the index is rebuilt from packs. The all-zeros -# name is a valid but unreachable sha256 (nothing hashes to it), so older borg reads it as an -# unreadable fragment and falls back to the rebuild. -CHUNKINDEX_INVALIDATED_NAME = "0" * 64 +# Marker object in the config/ namespace recording that the chunk index is invalid. Written before +# deleting index fragments and removed once every fragment is gone. While present, the chunk index is +# rebuilt from the packs on next load, so an interrupted fragment deletion cannot leave a partial +# fragment set in use. +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 2c564fd1cb..5361f2bb21 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -707,11 +707,10 @@ def store_list(namespace): # stop and report that rather than continue. matters for partial checks too, whose runs can be # days apart (e.g. a weekend cron job). index_infos = store_list("index") - # the index-invalidated sentinel is not a real fragment and its content does not match its - # name; skip verification and warn rather than report it as corruption. - if any(info.name == CHUNKINDEX_INVALIDATED_NAME for info in index_infos): - logger.warning("chunk index was invalidated by an interrupted operation; it will be rebuilt on next use.") - index_infos = [info for info in index_infos if info.name != CHUNKINDEX_INVALIDATED_NAME] + # a marker in config/ records that the chunk index is invalid (an interrupted fragment + # deletion); it will be rebuilt on next use, so warn rather than verify the leftover fragments. + if any(info.name == CHUNKINDEX_INVALID_SENTINEL for info in store_list("config")): + logger.warning("chunk index is invalid (interrupted operation); it will be rebuilt on next use.") index_pi = ProgressIndicatorPercent(total=len(index_infos), msg="Checking index %3.0f%%", msgid="check.index") for info in index_infos: self._lock_refresh() diff --git a/src/borg/testsuite/archiver/compact_cmd_test.py b/src/borg/testsuite/archiver/compact_cmd_test.py index c2f7d82ec1..88e2bf5620 100644 --- a/src/borg/testsuite/archiver/compact_cmd_test.py +++ b/src/borg/testsuite/archiver/compact_cmd_test.py @@ -139,7 +139,7 @@ def interrupt(): # The objects were deleted, so no readable chunk index may still list them. repository = open_repository(archiver) with repository: - assert list_chunkindex_hashes(repository)[0] == [] + assert list_chunkindex_hashes(repository) == [] # A later backup of identical content must re-upload the deleted chunks, ... cmd(archiver, "create", "archive2", "input") @@ -193,7 +193,7 @@ def store_delete_then_interrupt(name, **kwargs): # every persisted index entry points at a pack that still exists repository = open_repository(archiver) with repository: - assert list_chunkindex_hashes(repository)[0] != [] + assert list_chunkindex_hashes(repository) != [] pack_names_after = {info.name for info in repository.store_list("packs")} assert 0 < len(pack_names_after) < len(pack_names_before) # some packs deleted, some left for id, entry in repository.chunks.iteritems(): diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index 816ad6f2c9..6cead48c87 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -18,10 +18,10 @@ list_chunkindex_hashes, read_chunkindex_from_repo, repack_chunkindex, - write_chunkindex_invalidated, + write_chunkindex_invalid, write_chunkindex_to_repo, ) -from ..constants import CHUNKINDEX_INVALIDATED_NAME +from ..constants import CHUNKINDEX_INVALID_SENTINEL from ..hashindex import ChunkIndex, ChunkIndexEntry from ..crypto.key import AESOCBKey from ..helpers import safe_ns @@ -163,7 +163,7 @@ def test_build_chunkindex_retries_on_vanished_fragment(tmp_path): ci = ChunkIndex() ci[h] = ChunkIndexEntry(ChunkIndex.F_NEW, 0, h, 0, 4) write_chunkindex_to_repo(repository, ci, incremental=False, force_write=True) - fragment_names = list_chunkindex_hashes(repository)[0] + fragment_names = list_chunkindex_hashes(repository) original_store_load = repository.store_load raced = False @@ -224,7 +224,7 @@ def test_chunkindex_cache_not_consolidated_on_access(tmp_path): ci = ChunkIndex() ci[h] = ChunkIndexEntry(ChunkIndex.F_NEW, 0, h, 0, 4) write_chunkindex_to_repo(repository, ci, incremental=False, force_write=True) - before = len(list_chunkindex_hashes(repository)[0]) + before = len(list_chunkindex_hashes(repository)) assert before > 1 cache = ChunksMixin() @@ -232,7 +232,7 @@ def test_chunkindex_cache_not_consolidated_on_access(tmp_path): index = cache.chunks # binds the repository index; must NOT collapse the fragments # fragments are left intact (no consolidation side effect) ... - assert len(list_chunkindex_hashes(repository)[0]) == before + assert len(list_chunkindex_hashes(repository)) == before # ... and the in-memory index still resolves every seeded chunk assert H(1) in index and H(2) in index @@ -268,7 +268,7 @@ def test_write_chunkindex_splits_full_write(tmp_path, monkeypatch): write_chunkindex_to_repo( repository, _make_chunkindex(keys), incremental=False, force_write=True, delete_other=True ) - frags = list_chunkindex_fragments(repository)[0] + frags = list_chunkindex_fragments(repository) # 7000 entries split by MAX=3000 -> 3 fragments (3000 + 3000 + 1000) counts = sorted(len(read_chunkindex_from_repo(repository, name)) for name, _ in frags) assert counts == [1000, 3000, 3000] @@ -286,9 +286,9 @@ def test_repack_defers_when_below_min_and_few_fragments(tmp_path, monkeypatch): delete_chunkindex_from_repo(repository) for j in range(3): # 3 fragments * 100 = 300 entries < MIN, count 3 <= cap 5 _seed_fragment(repository, j * 100, 100) - before = {name for name, _ in list_chunkindex_fragments(repository)[0]} + before = {name for name, _ in list_chunkindex_fragments(repository)} repack_chunkindex(repository) - after = {name for name, _ in list_chunkindex_fragments(repository)[0]} + after = {name for name, _ in list_chunkindex_fragments(repository)} assert after == before # nothing merged @@ -304,9 +304,9 @@ def test_repack_seals_when_smalls_reach_min(tmp_path, monkeypatch): all_keys = [] for j in range(5): # 5 * 300 = 1500 >= MIN all_keys += _seed_fragment(repository, j * 300, 300) - assert len(list_chunkindex_fragments(repository)[0]) == 5 + assert len(list_chunkindex_fragments(repository)) == 5 repack_chunkindex(repository) - frags = list_chunkindex_fragments(repository)[0] + frags = list_chunkindex_fragments(repository) assert len(frags) == 1 # 1500 entries, one fragment (< MAX) merged = read_chunkindex_from_repo(repository, frags[0][0]) assert len(merged) == 1500 @@ -325,9 +325,9 @@ def test_repack_cap_forces_merge_below_min(tmp_path, monkeypatch): all_keys = [] for j in range(6): # 6 * 50 = 300 < MIN, but count 6 > cap 5 all_keys += _seed_fragment(repository, j * 50, 50) - assert len(list_chunkindex_fragments(repository)[0]) == 6 + assert len(list_chunkindex_fragments(repository)) == 6 repack_chunkindex(repository) - frags = list_chunkindex_fragments(repository)[0] + frags = list_chunkindex_fragments(repository) assert len(frags) == 1 # merged into a single sub-MIN remainder merged = read_chunkindex_from_repo(repository, frags[0][0]) assert len(merged) == 300 @@ -344,7 +344,7 @@ def test_write_chunkindex_splits_incremental_write(tmp_path, monkeypatch): keys = [_ci_key(i) for i in range(1200)] # all F_NEW -> written by the incremental path write_chunkindex_to_repo(repository, _make_chunkindex(keys), incremental=True) counts = sorted( - len(read_chunkindex_from_repo(repository, name)) for name, _ in list_chunkindex_fragments(repository)[0] + len(read_chunkindex_from_repo(repository, name)) for name, _ in list_chunkindex_fragments(repository) ) assert counts == [200, 500, 500] # 1200 split by MAX=500 @@ -366,7 +366,7 @@ def test_write_chunkindex_deterministic_fragments(tmp_path, monkeypatch): delete_chunkindex_from_repo(repository) keys = [_ci_key(i) for i in (reversed(key_ints) if reverse else key_ints)] write_chunkindex_to_repo(repository, _make_chunkindex(keys), incremental=False, force_write=True) - frags = list_chunkindex_fragments(repository)[0] + frags = list_chunkindex_fragments(repository) hashes.append({name for name, _ in frags}) # keys are big-endian ints, so sorted key order == numeric order: the batches must # hold exactly the ranges [0..499], [500..999], [1000..1199]. @@ -406,7 +406,7 @@ def test_close_consolidates_fragments_across_sessions(tmp_path, monkeypatch): repository.flush() with Repository(loc, exclusive=True) as repository: - frags = list_chunkindex_fragments(repository)[0] + frags = list_chunkindex_fragments(repository) # without repack there would be one incremental fragment per session (plus creation's empty); # repack consolidates the small ones as they accumulate, so we end up with fewer. assert len(frags) < 5 @@ -427,7 +427,7 @@ def test_repack_leaves_sealed_untouched_and_reconstructs(tmp_path, monkeypatch): with Repository(repository_location, exclusive=True, create=True) as repository: delete_chunkindex_from_repo(repository) sealed_keys = _seed_fragment(repository, 0, 2000) # >= MIN -> sealed - sealed_hashes = {name for name, _ in list_chunkindex_fragments(repository)[0]} + sealed_hashes = {name for name, _ in list_chunkindex_fragments(repository)} assert len(sealed_hashes) == 1 small_keys = [] for j in range(3): # 3 * 400 = 1200 >= MIN -> will be merged @@ -435,7 +435,7 @@ def test_repack_leaves_sealed_untouched_and_reconstructs(tmp_path, monkeypatch): repack_chunkindex(repository) - frags = {name for name, _ in list_chunkindex_fragments(repository)[0]} + frags = {name for name, _ in list_chunkindex_fragments(repository)} assert sealed_hashes <= frags # the sealed fragment was not rewritten or deleted assert len(frags) == 2 # sealed one + the merged small ones @@ -464,58 +464,58 @@ def test_repack_idempotent_deletes_sources_when_merged_exists(tmp_path, monkeypa for j in range(5): # 5 * 300 = 1500 >= MIN -> merges into one sealed fragment all_keys += _seed_fragment(repository, j * 300, 300) repack_chunkindex(repository) - sealed = {name for name, _ in list_chunkindex_fragments(repository)[0]} + sealed = {name for name, _ in list_chunkindex_fragments(repository)} assert len(sealed) == 1 # the merged, sealed fragment # simulate a repack that stored the merged fragment but crashed before deleting the sources: # the sealed fragment is present, and the same small sources exist again alongside it. for j in range(5): _seed_fragment(repository, j * 300, 300) - assert len(list_chunkindex_fragments(repository)[0]) == 6 # sealed + 5 re-seeded small + assert len(list_chunkindex_fragments(repository)) == 6 # sealed + 5 re-seeded small repack_chunkindex(repository) # merged content already exists -> upload is dedupe-skipped - frags = {name for name, _ in list_chunkindex_fragments(repository)[0]} + frags = {name for name, _ in list_chunkindex_fragments(repository)} assert frags == sealed # sources deleted; only the (unchanged) sealed fragment remains merged = read_chunkindex_from_repo(repository, next(iter(frags))) assert set(merged) == set(all_keys) -def _sentinel_present(repository): - return any(info.name == CHUNKINDEX_INVALIDATED_NAME for info in repository.store_list("index")) +def _invalid_marker_present(repository): + return any(info.name == CHUNKINDEX_INVALID_SENTINEL for info in repository.store_list("config")) -def test_invalidated_sentinel_forces_rebuild(tmp_path): - """With the sentinel present, build discards leftover fragments instead of trusting them.""" +def test_invalid_marker_forces_rebuild(tmp_path): + """With the marker present, build discards leftover fragments instead of trusting them.""" loc = os.fspath(tmp_path / "repository") with Repository(loc, exclusive=True, create=True) as repository: - # a leftover fragment plus the sentinel, as an interrupted deletion would leave them. + # a leftover fragment plus the marker, as an interrupted deletion would leave them. _seed_fragment(repository, 5000, 3) - write_chunkindex_invalidated(repository) - assert _sentinel_present(repository) + write_chunkindex_invalid(repository) + assert _invalid_marker_present(repository) chunks = build_chunkindex_from_repo(repository) # the leftover entry is not merged (the repo has no packs, so a correct rebuild is empty). assert _ci_key(5000) not in chunks - # the roll-forward removed both the leftover fragments and the sentinel. - assert not _sentinel_present(repository) - assert list_chunkindex_fragments(repository)[0] == [] + # the roll-forward removed both the leftover fragments and the marker. + assert not _invalid_marker_present(repository) + assert list_chunkindex_fragments(repository) == [] -def test_delete_chunkindex_writes_then_removes_sentinel(tmp_path): - """An interrupted delete_chunkindex_from_repo leaves the sentinel behind.""" +def test_delete_chunkindex_writes_then_removes_marker(tmp_path): + """An interrupted delete_chunkindex_from_repo leaves the invalid marker behind.""" loc = os.fspath(tmp_path / "repository") with Repository(loc, exclusive=True, create=True) as repository: _seed_fragment(repository, 0, 3) _seed_fragment(repository, 100, 3) - assert not _sentinel_present(repository) + assert not _invalid_marker_present(repository) real_delete = repository.store.delete calls = {"n": 0} def failing_delete(name, **kwargs): - # let the sentinel write happen, then fail on the first real fragment deletion. - if name.startswith("index/") and name != f"index/{CHUNKINDEX_INVALIDATED_NAME}": + # let the marker write happen, then fail on the first fragment deletion. + if name.startswith("index/"): calls["n"] += 1 if calls["n"] == 1: raise OSError("simulated crash mid-delete") @@ -525,16 +525,16 @@ def failing_delete(name, **kwargs): with pytest.raises(OSError): delete_chunkindex_from_repo(repository) repository.store.delete = real_delete - # the sentinel was written before the (failed) deletion and is still present. - assert _sentinel_present(repository) + # the marker was written before the (failed) deletion and is still present. + assert _invalid_marker_present(repository) - # a fresh build sees the sentinel, rolls the deletion forward, and rebuilds cleanly. + # a fresh build sees the marker, rolls the deletion forward, and rebuilds cleanly. build_chunkindex_from_repo(repository) - assert not _sentinel_present(repository) + assert not _invalid_marker_present(repository) -def test_repack_skips_when_invalidated(tmp_path, monkeypatch): - """Repack does nothing while the index is invalidated (a rebuild will supersede it).""" +def test_repack_skips_when_invalid(tmp_path, monkeypatch): + """Repack does nothing while the index is invalid (a rebuild will supersede it).""" monkeypatch.setattr(cache_mod, "CHUNKINDEX_FRAGMENT_ENTRIES_MIN", 1000) monkeypatch.setattr(cache_mod, "CHUNKINDEX_SMALL_FRAGMENT_CAP", 2) loc = os.fspath(tmp_path / "repository") @@ -542,21 +542,13 @@ def test_repack_skips_when_invalidated(tmp_path, monkeypatch): delete_chunkindex_from_repo(repository) for j in range(4): _seed_fragment(repository, j * 100, 100) - write_chunkindex_invalidated(repository) - before = {name for name, _ in list_chunkindex_fragments(repository)[0]} + write_chunkindex_invalid(repository) + before = {name for name, _ in list_chunkindex_fragments(repository)} repack_chunkindex(repository) - after = {name for name, _ in list_chunkindex_fragments(repository)[0]} + after = {name for name, _ in list_chunkindex_fragments(repository)} assert before == after # unchanged: repack bailed out -def test_read_sentinel_returns_none(tmp_path): - """Reading the sentinel as a fragment returns None (the old-borg graceful-degradation path).""" - loc = os.fspath(tmp_path / "repository") - with Repository(loc, exclusive=True, create=True) as repository: - write_chunkindex_invalidated(repository) - assert read_chunkindex_from_repo(repository, CHUNKINDEX_INVALIDATED_NAME) is None - - def test_files_cache_save_tolerates_missing_chunk(tmp_path, monkeypatch): """A files-cache entry whose chunk vanished from the index is dropped, not fatal. diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index bccc557063..4a1f139c10 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -119,7 +119,7 @@ def test_chunk_index_persisted_on_close(tmp_path): # reopen and read the cached fragments straight from disk with Repository(location, exclusive=True) as repository: persisted = ChunkIndex() - for hash in list_chunkindex_hashes(repository)[0]: + for hash in list_chunkindex_hashes(repository): fragment = read_chunkindex_from_repo(repository, hash) if fragment is not None: for k, v in fragment.items(): @@ -969,6 +969,19 @@ def test_check_detects_index_corruption(tmp_path): assert repository.check(repair=False) is False # mismatch between content hash and name detected +def test_check_warns_on_invalid_chunk_index(tmp_path, caplog): + # a config/ marker records that the chunk index is invalid; check warns but does not fail, + # since the index is not part of the repository's object integrity. + import logging + from ..constants import CHUNKINDEX_INVALID_SENTINEL + + with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository: + repository.store_store(f"config/{CHUNKINDEX_INVALID_SENTINEL}", b"") + with caplog.at_level(logging.WARNING): + assert repository.check(repair=False) is True + assert "chunk index is invalid" in caplog.text + + def test_check_intact_multi_object_pack_passes(tmp_path): # An intact pack with several objects passes: it is hashed as a whole, so the object count # does not matter. From d344e6e253d809fbbe6886b91f93ad58de37c64b Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 15 Jul 2026 10:04:47 +0530 Subject: [PATCH 3/5] cache: deduplicate the chunk-index-invalid check into chunkindex_is_invalid() --- src/borg/repository.py | 8 +++++--- src/borg/testsuite/cache_test.py | 16 ++++++---------- src/borg/testsuite/repository_test.py | 8 ++++---- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index 5361f2bb21..86432218f5 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -707,9 +707,11 @@ def store_list(namespace): # stop and report that rather than continue. matters for partial checks too, whose runs can be # days apart (e.g. a weekend cron job). index_infos = store_list("index") - # a marker in config/ records that the chunk index is invalid (an interrupted fragment - # deletion); it will be rebuilt on next use, so warn rather than verify the leftover fragments. - if any(info.name == CHUNKINDEX_INVALID_SENTINEL for info in store_list("config")): + # an invalid chunk index means an interrupted fragment deletion; it will be rebuilt on next + # use, so warn rather than verify the leftover fragments. + from .cache import chunkindex_is_invalid + + if chunkindex_is_invalid(self): logger.warning("chunk index is invalid (interrupted operation); it will be rebuilt on next use.") index_pi = ProgressIndicatorPercent(total=len(index_infos), msg="Checking index %3.0f%%", msgid="check.index") for info in index_infos: diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index 6cead48c87..8e43e1cdec 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -13,6 +13,7 @@ ChunksMixin, FileCacheEntry, build_chunkindex_from_repo, + chunkindex_is_invalid, delete_chunkindex_from_repo, list_chunkindex_fragments, list_chunkindex_hashes, @@ -21,7 +22,6 @@ write_chunkindex_invalid, write_chunkindex_to_repo, ) -from ..constants import CHUNKINDEX_INVALID_SENTINEL from ..hashindex import ChunkIndex, ChunkIndexEntry from ..crypto.key import AESOCBKey from ..helpers import safe_ns @@ -481,10 +481,6 @@ def test_repack_idempotent_deletes_sources_when_merged_exists(tmp_path, monkeypa assert set(merged) == set(all_keys) -def _invalid_marker_present(repository): - return any(info.name == CHUNKINDEX_INVALID_SENTINEL for info in repository.store_list("config")) - - def test_invalid_marker_forces_rebuild(tmp_path): """With the marker present, build discards leftover fragments instead of trusting them.""" loc = os.fspath(tmp_path / "repository") @@ -492,13 +488,13 @@ def test_invalid_marker_forces_rebuild(tmp_path): # a leftover fragment plus the marker, as an interrupted deletion would leave them. _seed_fragment(repository, 5000, 3) write_chunkindex_invalid(repository) - assert _invalid_marker_present(repository) + assert chunkindex_is_invalid(repository) chunks = build_chunkindex_from_repo(repository) # the leftover entry is not merged (the repo has no packs, so a correct rebuild is empty). assert _ci_key(5000) not in chunks # the roll-forward removed both the leftover fragments and the marker. - assert not _invalid_marker_present(repository) + assert not chunkindex_is_invalid(repository) assert list_chunkindex_fragments(repository) == [] @@ -508,7 +504,7 @@ def test_delete_chunkindex_writes_then_removes_marker(tmp_path): with Repository(loc, exclusive=True, create=True) as repository: _seed_fragment(repository, 0, 3) _seed_fragment(repository, 100, 3) - assert not _invalid_marker_present(repository) + assert not chunkindex_is_invalid(repository) real_delete = repository.store.delete calls = {"n": 0} @@ -526,11 +522,11 @@ def failing_delete(name, **kwargs): delete_chunkindex_from_repo(repository) repository.store.delete = real_delete # the marker was written before the (failed) deletion and is still present. - assert _invalid_marker_present(repository) + assert chunkindex_is_invalid(repository) # a fresh build sees the marker, rolls the deletion forward, and rebuilds cleanly. build_chunkindex_from_repo(repository) - assert not _invalid_marker_present(repository) + assert not chunkindex_is_invalid(repository) def test_repack_skips_when_invalid(tmp_path, monkeypatch): diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 4a1f139c10..8de4d86f6b 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -970,13 +970,13 @@ def test_check_detects_index_corruption(tmp_path): def test_check_warns_on_invalid_chunk_index(tmp_path, caplog): - # a config/ marker records that the chunk index is invalid; check warns but does not fail, - # since the index is not part of the repository's object integrity. + # check warns about an invalid chunk index but does not fail, since the index is not part of + # the repository's object integrity. import logging - from ..constants import CHUNKINDEX_INVALID_SENTINEL + from ..cache import write_chunkindex_invalid with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository: - repository.store_store(f"config/{CHUNKINDEX_INVALID_SENTINEL}", b"") + write_chunkindex_invalid(repository) with caplog.at_level(logging.WARNING): assert repository.check(repair=False) is True assert "chunk index is invalid" in caplog.text From ea1d86d6998c1fc1b367249306c7eb96efdd51f3 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Fri, 17 Jul 2026 16:01:09 +0530 Subject: [PATCH 4/5] cache: move the chunk-index-invalid marker from the config/ to the cache/ namespace --- src/borg/cache.py | 6 +++--- src/borg/constants.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index c73f636aaf..8560b20026 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -582,7 +582,7 @@ def chunkindex_is_invalid(repository): store_list() bypasses the borgstore cache, so this stays correct even for a cache-backed namespace. """ - return any(ItemInfo(*info).name == CHUNKINDEX_INVALID_SENTINEL for info in repository.store_list("config")) + return any(ItemInfo(*info).name == CHUNKINDEX_INVALID_SENTINEL for info in repository.store_list("cache")) def write_chunkindex_invalid(repository): @@ -590,13 +590,13 @@ def write_chunkindex_invalid(repository): If the deletion is interrupted, the marker remains and the index is rebuilt on next load. """ - repository.store_store(f"config/{CHUNKINDEX_INVALID_SENTINEL}", b"") + 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.""" try: - repository.store_delete(f"config/{CHUNKINDEX_INVALID_SENTINEL}") + repository.store_delete(f"cache/{CHUNKINDEX_INVALID_SENTINEL}") except StoreObjectNotFound: pass diff --git a/src/borg/constants.py b/src/borg/constants.py index effda55c37..8dc525f1f6 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -114,7 +114,7 @@ # 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 config/ namespace recording that the chunk index is invalid. Written before +# Marker object in the cache/ namespace recording that the chunk index is invalid. Written before # deleting index fragments and removed once every fragment is gone. While present, the chunk index is # rebuilt from the packs on next load, so an interrupted fragment deletion cannot leave a partial # fragment set in use. From 629d4405e6e45b551d3a6b0f6a1cf4722a4517fc Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Tue, 21 Jul 2026 00:11:15 +0530 Subject: [PATCH 5/5] cache: document that the chunk-index-invalid marker must not be removed on its own --- src/borg/constants.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/borg/constants.py b/src/borg/constants.py index 8dc525f1f6..30bcd62773 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -114,10 +114,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 recording that the chunk index is invalid. Written before -# deleting index fragments and removed once every fragment is gone. While present, the chunk index is -# rebuilt from the packs on next load, so an interrupted fragment deletion cannot leave a partial -# fragment set in use. +# 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. CHUNKINDEX_INVALID_SENTINEL = "chunkindex-invalid" FD_MAX_AGE = 4 * 60 # 4 minutes