From c82a2800c41261607a51af7327f7751143ebbc71 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 28 Aug 2026 22:08:09 +0200 Subject: [PATCH] write chunkindex fragment(s) before the archive pointer, fixes #10239 The archive pointer write (archives/) is the commit point of a backup. Previously, the chunkindex fragment covering the session's new chunks was only written at cache close (and every 600s during the backup), i.e. AFTER the pointer: a crash in between left a committed archive whose chunks were partly not covered by index/*, so reads (which are routed through the index) raised ObjectNotFound until "borg check" rebuilt the index. Now Archive.save() and Archive.set_meta() (rename/tag/copy) persist the index fragment(s) covering everything the session stored so far before writing the pointer, so a committed archive always has complete index coverage. A crash before the pointer write merely leaves an index fragment referencing uncommitted objects, which is harmless (compact/rebuild prunes it). This also covers import-tar, recreate and transfer, which commit archives through Archive.save(). The close-time write is now usually a no-op (no new chunks), but is kept for sessions that store chunks without committing an archive. Also: remove the packs.rst TODO about this and update the write-order steps to match the implementation. Co-Authored-By: Claude Fable 5 --- docs/internals/packs.rst | 13 +++++---- src/borg/archive.py | 7 +++++ src/borg/cache.py | 9 +++++++ .../testsuite/archiver/create_cmd_test.py | 27 +++++++++++++++++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/docs/internals/packs.rst b/docs/internals/packs.rst index 9c56fc9339..92ed7d8d56 100644 --- a/docs/internals/packs.rst +++ b/docs/internals/packs.rst @@ -169,16 +169,15 @@ entry carries the ``F_PENDING`` flag and its pack location is unresolved. crash before it leaves only unreferenced objects that ``borg compact`` reclaims. -.. TODO: the implementation currently writes the session's final index fragment at - cache close, i.e. after the archive pointer - see issue #10239. - Pack data must be stored before any archive pointer references it. The required write order is: -1. Store the pack file to ``packs/`` via borgstore. -2. Store the partial index file to ``index/`` (see :ref:`pack-index-namespace`). -3. Write the archive metadata object into a pack, then write the archive pointer - ``archives/``. This pointer write is the sole commit point. +1. Store the pack files to ``packs/`` via borgstore. The archive metadata + object goes into a (usually tiny) pack of its own, stored last. +2. Store index fragment(s) covering all objects the session stored -- the archive + metadata object included -- to ``index/`` (see :ref:`pack-index-namespace`). +3. Write the archive pointer ``archives/``. This pointer write is + the sole commit point. A crash between steps 1 and 2 leaves orphan pack files in ``packs/``. No archive references these chunks; ``borg compact`` removes them on the next run. diff --git a/src/borg/archive.py b/src/borg/archive.py index 84c9772d08..bc45bdad41 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -801,6 +801,11 @@ def save(self, name=None, comment=None, timestamp=None, stats=None, additional_m raise Error("%s - archive too big (issue #1473)!" % err_msg) else: raise + # the index fragment(s) covering this session's chunks must be stored before the archive + # pointer: the pointer write below is the commit point, and a committed archive must have + # complete index coverage. A crash before the pointer is written merely leaves an index + # fragment referencing uncommitted objects, which compact/rebuild prunes (#10239). + self.cache.write_chunks_index() self.manifest.archives.create(name, self.id, metadata.time) self.manifest.write() return metadata @@ -1149,6 +1154,8 @@ def set_meta(self, key, value): data = self.key.pack_metadata(metadata.as_dict()) new_id = self.key.id_hash(data) self.cache.add_chunk(new_id, {}, data, stats=self.stats, ro_type=ROBJ_ARCHIVE_META) + # index coverage must be complete before the archive pointer is written, see save(). + self.cache.write_chunks_index() self.manifest.archives.create(self.name, new_id, metadata.time, overwrite=True) self.id = new_id diff --git a/src/borg/cache.py b/src/borg/cache.py index 6977ac2735..0e4241a99c 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -1169,6 +1169,15 @@ def _maybe_write_chunks_index(self, now, force=False, clear=False): write_chunkindex_to_repo(self.repository, self._chunks, clear=clear) self.chunks_index_last_write = now + def write_chunks_index(self): + """Flush the pack writer and persist the session's new chunks as index fragment(s) now. + + Called before an archive pointer is written (the commit point), so that a committed + archive always has complete index coverage, see #10239. Also resets the periodic index + write timer. + """ + self._maybe_write_chunks_index(datetime.now(UTC), force=True) + def refresh_lock(self, now): if now > self.last_refresh_dt + self.refresh_td: # the repository lock needs to get refreshed regularly, or it will be killed as stale. diff --git a/src/borg/testsuite/archiver/create_cmd_test.py b/src/borg/testsuite/archiver/create_cmd_test.py index 00daa8d0eb..6750177cea 100644 --- a/src/borg/testsuite/archiver/create_cmd_test.py +++ b/src/borg/testsuite/archiver/create_cmd_test.py @@ -2008,3 +2008,30 @@ def test_create_digests_invalid_algo(archivers, request): cmd(archiver, "repo-create", RK_ENCRYPTION) output = cmd(archiver, "create", "--digests=nosuchhash", "test", "input", exit_code=2) assert "digests must be" in output and "blake3" in output # the error lists the valid algorithms + + +def test_chunkindex_covers_committed_archive(archiver, monkeypatch): + """The index fragments covering a session's chunks are written before the archive pointer (#10239). + + Simulate borg dying right after the commit point (the archives/* pointer write): the close-time + index write does not happen then. As reads are routed through the chunk index, the committed + archive is only readable if index fragments covering all of its chunks were written before the + pointer. + """ + from ...cache import ChunksMixin + + cmd(archiver, "repo-create", RK_ENCRYPTION) + create_regular_file(archiver.input_path, "file1", size=1024 * 80) + orig_maybe_write = ChunksMixin._maybe_write_chunks_index + + def no_close_time_write(self, now, force=False, clear=False): + if clear: + return # skip the cache-close index write, like a crash right after committing + orig_maybe_write(self, now, force=force, clear=clear) + + with monkeypatch.context() as m: + m.setattr(ChunksMixin, "_maybe_write_chunks_index", no_close_time_write) + cmd(archiver, "create", "test", "input") + with changedir("output"): + cmd(archiver, "extract", "test") + assert_dirs_equal("input", "output/input")