From f929ae81aa1847cd006256ff258da006a3247a21 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 13 Jul 2026 00:02:39 +0530 Subject: [PATCH 1/3] cache: flush pack writer before periodic chunk-index write (#9900) Otherwise buffered (F_PENDING) chunks are persisted with the placeholder pack_id, and clearing F_NEW prevents their real location from being written. --- src/borg/cache.py | 3 +++ src/borg/testsuite/cache_test.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/borg/cache.py b/src/borg/cache.py index 4872871311..4237b94ee4 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -913,6 +913,9 @@ def add_chunk( def _maybe_write_chunks_index(self, now, force=False, clear=False): if force or now > self.chunks_index_last_write + self.chunks_index_write_td: if self._chunks is not None: + # flush the pack writer first, so buffered chunks get their real pack location + # in the index; the write only persists a placeholder location otherwise (#9900). + self.repository.flush() write_chunkindex_to_repo(self.repository, self._chunks, clear=clear) self.chunks_index_last_write = now diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index 3bdc39ff9d..2f8e5b14d5 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -1,5 +1,6 @@ import os import time +from datetime import UTC, datetime import pytest @@ -19,6 +20,7 @@ repack_chunkindex, write_chunkindex_to_repo, ) +from ..constants import UNKNOWN_BYTES32 from ..hashindex import ChunkIndex, ChunkIndexEntry from ..crypto.key import AESOCBKey from ..helpers import safe_ns @@ -66,6 +68,19 @@ def test_existing_reuse_after_add_chunk(self, cache): assert cache.add_chunk(H(1), {}, b"5678", stats=Statistics()) == (H(1), 4) assert cache.reuse_chunk(H(1), 4, Statistics()) == (H(1), 4) + def test_periodic_index_write_flushes_pending(self, repository, key, manifest): + # #9900: the periodic index write must flush the pack writer first, so a buffered chunk is + # persisted with its real pack location instead of the UNKNOWN_BYTES32 placeholder. + cache = AdHocWithFilesCache(manifest) + cache.add_chunk(H(7), {}, b"h1-payload", stats=Statistics()) + assert repository.chunks.is_pending(H(7)) # H(7) is buffered, not yet in a pack + # force=True runs the same write the 600s timer triggers: + cache._maybe_write_chunks_index(datetime.now(UTC), force=True) + # rebuild the index from the persisted fragments and check H(7)'s stored location: + index = build_chunkindex_from_repo(repository) + assert H(7) in index + assert index[H(7)].pack_id != UNKNOWN_BYTES32 + def test_files_cache(self, cache): st = os.stat(".") assert cache.file_known_and_unchanged(b"foo", bytes(32), st) == (False, None) From 7e65316d5865f92265626513337a7c606186d880 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 13 Jul 2026 00:18:26 +0530 Subject: [PATCH 2/3] cache: assert real pack_id before serializing chunk-index entry (#9900) --- src/borg/cache.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index 4237b94ee4..afbbd360ca 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -21,7 +21,7 @@ from borgstore.store import ItemInfo 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_FRAGMENT_ENTRIES_MIN, CHUNKINDEX_FRAGMENT_ENTRIES_MAX, UNKNOWN_BYTES32 from .constants import CHUNKINDEX_SMALL_FRAGMENT_CAP, CHUNKINDEX_MERGE_ATTEMPTS from .hashindex import ChunkIndex, ChunkIndexEntry, ChunkIndexEntryFormat from .helpers import get_cache_dir @@ -653,8 +653,12 @@ def write_chunkindex_to_repo( # pre-size the temporary table to this batch so filling it does not repeatedly rehash: batch = ChunkIndex(usable=len(batch_keys) or None) for key in batch_keys: + entry = chunks[key] + # a chunk still buffered in the pack writer has UNKNOWN_BYTES32 as its pack_id; + # only serialize an entry once its pack is written and its pack_id is real (#9900). + assert entry.pack_id != UNKNOWN_BYTES32, f"chunk {bin_to_hex(key)} has no pack location" # for now, we don't want to serialize the flags or the size: - batch[key] = chunks[key]._replace(flags=ChunkIndex.F_NONE, size=0) + batch[key] = entry._replace(flags=ChunkIndex.F_NONE, size=0) new_hash, stored = _store_chunkindex_fragment(repository, batch, stored_hashes, force_write=force_write) batch.clear() # free memory of the temporary table new_hashes.add(new_hash) @@ -913,8 +917,8 @@ def add_chunk( def _maybe_write_chunks_index(self, now, force=False, clear=False): if force or now > self.chunks_index_last_write + self.chunks_index_write_td: if self._chunks is not None: - # flush the pack writer first, so buffered chunks get their real pack location - # in the index; the write only persists a placeholder location otherwise (#9900). + # flush the pack writer first, so buffered chunks get their real pack location; + # until their pack is written their pack_id is the UNKNOWN_BYTES32 placeholder (#9900). self.repository.flush() write_chunkindex_to_repo(self.repository, self._chunks, clear=clear) self.chunks_index_last_write = now From 7da670c115834bd184a72cb61c24ea05971f71aa Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 13 Jul 2026 15:45:00 +0530 Subject: [PATCH 3/3] cache: assert F_PENDING instead of the UNKNOWN_BYTES32 sentinel when serializing chunk-index entries Also drop the now-redundant Cache.close flush, since _maybe_write_chunks_index flushes on the force path. --- src/borg/cache.py | 12 +++++------- src/borg/testsuite/cache_test.py | 13 +++++++------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index afbbd360ca..32b3a41913 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -21,7 +21,7 @@ from borgstore.store import ItemInfo 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, UNKNOWN_BYTES32 +from .constants import CHUNKINDEX_FRAGMENT_ENTRIES_MIN, CHUNKINDEX_FRAGMENT_ENTRIES_MAX from .constants import CHUNKINDEX_SMALL_FRAGMENT_CAP, CHUNKINDEX_MERGE_ATTEMPTS from .hashindex import ChunkIndex, ChunkIndexEntry, ChunkIndexEntryFormat from .helpers import get_cache_dir @@ -654,9 +654,9 @@ def write_chunkindex_to_repo( batch = ChunkIndex(usable=len(batch_keys) or None) for key in batch_keys: entry = chunks[key] - # a chunk still buffered in the pack writer has UNKNOWN_BYTES32 as its pack_id; - # only serialize an entry once its pack is written and its pack_id is real (#9900). - assert entry.pack_id != UNKNOWN_BYTES32, f"chunk {bin_to_hex(key)} has no pack location" + # a chunk still buffered in the pack writer has F_PENDING set: its pack location + # is not resolved yet. only serialize an entry once its pack is written (#9900). + assert not (entry.flags & ChunkIndex.F_PENDING), f"chunk {bin_to_hex(key)} has no pack location yet" # for now, we don't want to serialize the flags or the size: batch[key] = entry._replace(flags=ChunkIndex.F_NONE, size=0) new_hash, stored = _store_chunkindex_fragment(repository, batch, stored_hashes, force_write=force_write) @@ -918,7 +918,7 @@ def _maybe_write_chunks_index(self, now, force=False, clear=False): if force or now > self.chunks_index_last_write + self.chunks_index_write_td: if self._chunks is not None: # flush the pack writer first, so buffered chunks get their real pack location; - # until their pack is written their pack_id is the UNKNOWN_BYTES32 placeholder (#9900). + # until their pack is written they are still F_PENDING with no location (#9900). self.repository.flush() write_chunkindex_to_repo(self.repository, self._chunks, clear=clear) self.chunks_index_last_write = now @@ -1011,8 +1011,6 @@ def open(self): def close(self): self.security_manager.save(self.manifest, self.key) pi = ProgressIndicatorMessage(msgid="cache.close") - if self._chunks is not None: - self.repository.flush() if self._files is not None: pi.output("Saving files cache") integrity_data = self._write_files_cache(self._files) diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index 2f8e5b14d5..2a109a259f 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -20,7 +20,6 @@ repack_chunkindex, write_chunkindex_to_repo, ) -from ..constants import UNKNOWN_BYTES32 from ..hashindex import ChunkIndex, ChunkIndexEntry from ..crypto.key import AESOCBKey from ..helpers import safe_ns @@ -68,18 +67,20 @@ def test_existing_reuse_after_add_chunk(self, cache): assert cache.add_chunk(H(1), {}, b"5678", stats=Statistics()) == (H(1), 4) assert cache.reuse_chunk(H(1), 4, Statistics()) == (H(1), 4) - def test_periodic_index_write_flushes_pending(self, repository, key, manifest): + def test_periodic_index_write_flushes_pending(self, repository, cache): # #9900: the periodic index write must flush the pack writer first, so a buffered chunk is - # persisted with its real pack location instead of the UNKNOWN_BYTES32 placeholder. - cache = AdHocWithFilesCache(manifest) + # persisted with its real pack location rather than the unresolved F_PENDING placeholder. cache.add_chunk(H(7), {}, b"h1-payload", stats=Statistics()) assert repository.chunks.is_pending(H(7)) # H(7) is buffered, not yet in a pack # force=True runs the same write the 600s timer triggers: cache._maybe_write_chunks_index(datetime.now(UTC), force=True) - # rebuild the index from the persisted fragments and check H(7)'s stored location: + # rebuild the index from the persisted fragments and check H(7)'s stored location matches + # the real (flushed) location, not the placeholder: index = build_chunkindex_from_repo(repository) assert H(7) in index - assert index[H(7)].pack_id != UNKNOWN_BYTES32 + # the flush resolved H(7)'s location, so it is no longer pending/placeholder: + assert not repository.chunks.is_pending(H(7)) + assert index[H(7)].pack_id == repository.chunks[H(7)].pack_id def test_files_cache(self, cache): st = os.stat(".")