From 0ac9addcd51e92891fa94a092b605e31831acb1d Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Tue, 22 Sep 2026 12:44:34 +0530 Subject: [PATCH] check: stop the index rebuild per object, stop read-only checks within an archive An interrupted --repair index rebuild no longer reports the index as rebuilt, finish() warns that its rebuild can not be interrupted, and ChunkIndexRebuildInterrupted is listed in frontends.rst. --- docs/internals/frontends.rst | 2 + src/borg/archive.py | 15 ++++-- src/borg/archiver/check_cmd.py | 18 ++++--- src/borg/cache.py | 17 +++--- src/borg/repository.py | 4 +- src/borg/testsuite/archiver/check_cmd_test.py | 52 ++++++++++++++---- src/borg/testsuite/cache_test.py | 54 ++++++++++++------- src/borg/testsuite/repository_test.py | 19 ++++--- 8 files changed, 129 insertions(+), 52 deletions(-) diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 5d10ee738..ff3810998 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -814,6 +814,8 @@ Errors Buffer.MemoryLimitExceeded rc: 2 traceback: no Requested buffer size {} is above the limit of {}. + ChunkIndexRebuildInterrupted rc: 2 traceback: no + Got Ctrl-C / SIGINT: the chunk index rebuild was interrupted. EfficientCollectionQueue.SizeUnderflow rc: 2 traceback: no Could not pop the first {} elements; collection only has {} elements. RTError rc: 2 traceback: no diff --git a/src/borg/archive.py b/src/borg/archive.py index e74ff2407..80b5501c0 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2315,7 +2315,7 @@ class ArchiveChecker: # dropped content is a check finding, with or without --repair. on_drop=self.note_dropped_objects, write_immediately=False, - # Ctrl-C ends the rebuild after the current pack and aborts the check, #10042. + # Ctrl-C aborts the rebuild and with it the check, #10042. interruptible=True, ) # clear F_NEW (entry not in the index/ fragments yet), so Repository.close() does not store @@ -2734,7 +2734,7 @@ class ArchiveChecker: try: for i, info in enumerate(archive_infos): if sig_int: - # Break only between archives, as --repair rewrites each archive as a whole. + # --repair rewrites each archive as a whole, so with --repair the check stops only here. break pi.show(i) archive_id, archive_id_hex = info.id, bin_to_hex(info.id) @@ -2773,6 +2773,9 @@ class ArchiveChecker: items_buffer = ChunkBuffer(self.key) items_buffer.write_chunk = add_callback for item in robust_iterator(archive): + if sig_int and not self.repair: + # without --repair the archive is only read, so the check also stops within it. + break if "chunks" in item: verify_file_chunks(info.name, item) items_buffer.add(item) @@ -2806,9 +2809,15 @@ class ArchiveChecker: write_chunkindex_invalid(self.repository) self.repository.invalidate_chunk_index() self.chunks = None - logger.info("Rebuilding and writing the repository chunks index.") # Runs to completion, also after a Ctrl-C: delete_chunkindex_invalid() below declares # the stored index to match the packs, which holds only once every pack was indexed. + if sig_int: + logger.warning( + "Rebuilding and writing the repository chunks index. " + "This reads every pack and can not be interrupted." + ) + else: + logger.info("Rebuilding and writing the repository chunks index.") build_chunkindex_from_repo( self.repository, slow_rebuild=True, diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 677567c15..0685ae021 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -214,16 +214,18 @@ class CheckMixIn: If the ``borg check`` process receives a SIGINT signal (Ctrl-C), it stops at the next safe boundary, leaving the repository and its chunk index in a consistent state. The repository check stops after the current pack; ``--verify-data`` and - ``--find-lost-archives`` stop after the current chunk; a ``--repair`` archive check - stops between whole archives. Results recorded before the interrupt are kept, so a later - check does not re-verify those packs until they are due again. With ``--repair``, an - interrupted archive check may leave some archives already repaired and others not yet - processed, so run ``borg check --repair`` again to finish. + ``--find-lost-archives`` stop after the current chunk; the archive check stops after the + current archive item, with ``--repair`` between whole archives. Results recorded before + the interrupt are kept, so a later check does not re-verify those packs until they are + due again. With ``--repair``, an interrupted archive check may leave some archives already + repaired and others not yet processed, so run ``borg check --repair`` again to finish. ``borg check`` rebuilds the chunk index from the packs when ``--repair`` is given or when - the stored index cannot be used. Ctrl-C ends that rebuild after the current pack and - discards it: a partial index lacks chunks that are still in the repository and would report - them as lost. The stored index is left unchanged, so a later run rebuilds it from scratch. + the stored index cannot be used. Ctrl-C stops that rebuild after the current object and + discards the partial index: it lacks chunks that are still in the repository, so the check + would report them as lost. After a ``--repair`` that stored or deleted chunks, borg rebuilds and + stores the chunk index once more; that rebuild always runs to completion, also after a + Ctrl-C, and reads every pack. About repair mode +++++++++++++++++ diff --git a/src/borg/cache.py b/src/borg/cache.py index 498403035..64a1e2d5a 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -800,7 +800,7 @@ class CorruptChunkIndexFragment(Exception): class ChunkIndexRebuildInterrupted(Error): - """Got Ctrl-C / SIGINT: the chunk index rebuild was interrupted; the stored chunk index was left unchanged.""" + """Got Ctrl-C / SIGINT: the chunk index rebuild was interrupted.""" def read_chunkindex_from_repo(repository, hash): @@ -899,8 +899,8 @@ def build_chunkindex_from_repo( # the rebuild skips the objects that fail it; without one, a corrupt object header raises CorruptPack. # on_drop: a callable or None, passed to PackReader.iter_headers, called once per byte range the # validating walk skips. - # interruptible: on Ctrl-C / SIGINT, end the rebuild after the current pack and raise - # ChunkIndexRebuildInterrupted instead of returning an index. + # interruptible: on Ctrl-C / SIGINT, stop the pack walk before the next object, discard the index + # built so far (it lacks the chunks of the packs not walked yet) and raise ChunkIndexRebuildInterrupted. assert not (slow_rebuild and fragments_only) assert not (fragments_only and write_immediately) # fragments_only never writes to the repo # first, try to build a fresh, mostly complete chunk index from centrally stored index fragments: @@ -990,14 +990,17 @@ def build_chunkindex_from_repo( total=len(pack_infos), msg="Rebuilding chunk index %3.0f%%", msgid="cache.build_chunkindex_from_repo" ) headers_parsed = 0 - for packs_done, info in enumerate(pack_infos): + + def stop_if_interrupted(packs_done): + # packs_done: the number of packs indexed completely. if interruptible and sig_int: - # A partial index lacks chunks that are still in the packs: a check would report them as - # lost and --repair would drop the archives' references to them. logger.info(f"Chunk index rebuild interrupted after {packs_done} of {len(pack_infos)} packs.") chunks.clear() pi.finish() raise ChunkIndexRebuildInterrupted + + for packs_done, info in enumerate(pack_infos): + stop_if_interrupted(packs_done) # PackReader uses the store directly, so refresh the lock here; a full rebuild can be slow. repository._lock_refresh() pi.show(increase=1) @@ -1005,6 +1008,8 @@ def build_chunkindex_from_repo( reader = PackReader(repository.store, pack_id) try: for chunk_id, obj_offset, obj_size in reader.iter_headers(validate=validate, on_drop=on_drop): + # every object header is a store request, so also stop within a pack. + stop_if_interrupted(packs_done) num_chunks += 1 chunks[chunk_id] = ChunkIndexEntry( flags=init_flags, size=0, pack_id=pack_id, obj_offset=obj_offset, obj_size=obj_size diff --git a/src/borg/repository.py b/src/borg/repository.py index 738c1c62d..2445bee9b 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1596,8 +1596,8 @@ class Repository: interruptible=True, ) except ChunkIndexRebuildInterrupted: - # the rebuild stores the index after its pack loop, so the corrupt fragments are - # still there and the next use rebuilds from the packs. + # nothing was stored: the corrupt fragments stay, so the next use rebuilds from the packs. + drops = 0 # counted by the discarded rebuild, which covered only a part of the packs logger.warning("Index rebuild interrupted; the index stays corrupt and is rebuilt on next use.") else: self.invalidate_chunk_index() # the rebuilt index is persisted; drop the in-memory copy diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 67a340521..7fc449724 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -186,9 +186,9 @@ def test_check_repair_soft_interrupt(archivers, request, monkeypatch): def test_check_repair_interrupt_during_index_rebuild(archivers, request, monkeypatch): - """A --repair archive check rebuilds the chunk index before it reads any archive. A Ctrl-C ends that - rebuild at the pack boundary and raises, the stored index keeps the fragments it had, and a second - --repair completes the check, #10042.""" + """A --repair archive check rebuilds the chunk index before it reads any archive. A Ctrl-C stops that + rebuild and raises, the stored index keeps the fragments it had, and a second --repair completes the + check, #10042.""" archiver = request.getfixturevalue(archivers) check_cmd_setup(archiver) # produces several packs @@ -197,9 +197,8 @@ def test_check_repair_interrupt_during_index_rebuild(archivers, request, monkeyp def iter_headers_then_interrupt(self, **kwargs): packs_read.append(self.pack_id) - headers = list(orig_iter_headers(self, **kwargs)) - sig_int._sig_int_triggered = True # one Ctrl-C after the first pack was indexed - return iter(headers) + yield from orig_iter_headers(self, **kwargs) + sig_int._sig_int_triggered = True # one Ctrl-C after the first pack was walked with Repository(archiver.repository_path, exclusive=True) as repository: assert len(repository.store_list("packs")) > 1 # there is a pack boundary to stop at @@ -220,10 +219,13 @@ def test_check_repair_interrupt_during_index_rebuild(archivers, request, monkeyp cmd(archiver, "check", exit_code=0) -def test_check_repair_finish_completes_index_rebuild_after_interrupt(archivers, request, monkeypatch): +def test_check_repair_finish_completes_index_rebuild_after_interrupt(archiver, monkeypatch, capsys): """finish() runs with sig_int already set, #9850: its chunk index rebuild walks every pack and stores - an index that matches them, so the invalid marker is cleared and a plain check passes afterwards.""" - archiver = request.getfixturevalue(archivers) + an index that matches them, so the invalid marker is cleared and a plain check passes afterwards. + It warns that this rebuild can not be interrupted.""" + # local-only: this patches in-process internals, including check_cmd_setup's small ChunkBuffer.BUFFER_SIZE. + # With the default buffer size an archive's item metadata is a single chunk, which the repair rewrites to + # the same id, so it stores nothing and finish() skips its index rebuild. check_cmd_setup(archiver) # two archives orig_create = Archives.create @@ -268,11 +270,43 @@ def test_check_repair_finish_completes_index_rebuild_after_interrupt(archivers, # the repair stored re-packed item metadata chunks, so finish() takes its rebuild branch. assert checker.chunks_modified is True assert len(packs_read_in_finish) == pack_count + assert "This reads every pack and can not be interrupted." in capsys.readouterr().err with Repository(archiver.repository_path, exclusive=True) as repository: assert not chunkindex_is_invalid(repository) # finish() reached delete_chunkindex_invalid() cmd(archiver, "check", exit_code=0) # the stored index matches the packs +def test_check_interrupt_within_archive(archivers, request, monkeypatch): + """A check without --repair only reads the archives, so a Ctrl-C stops it after the current archive + item, #10042.""" + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + item_count = len(cmd(archiver, "list", "archive1", exit_code=0).splitlines()) + assert item_count > 1 # there is an item to stop before + + orig_add = ChunkBuffer.add + items_read = 0 + + def add_then_interrupt(self, item): + nonlocal items_read + items_read += 1 + sig_int._sig_int_triggered = True # one Ctrl-C after the first item + return orig_add(self, item) + + monkeypatch.setattr(ChunkBuffer, "add", add_then_interrupt) + try: + with Repository(archiver.repository_path, exclusive=True) as repository: + with pytest.raises(Error, match="Got Ctrl-C"): + ArchiveChecker().check(repository, sort_by="ts", format="{archive} {time} {id}") + finally: + sig_int._sig_int_triggered = False # reset the global flag for the following tests + # restore the real method; monkeypatch.undo() would also drop the autouse env (BORG_TESTONLY_WEAKEN_KDF). + monkeypatch.setattr(ChunkBuffer, "add", orig_add) + assert items_read == 1 # the check stopped within the first archive + + cmd(archiver, "check", exit_code=0) # the interrupted check changed nothing + + def test_check_interrupt_skips_archive_check(archivers, request, monkeypatch): """A Ctrl-C during the repository check makes a full `borg check` skip the archive check. do_check raises at the sig_int guard, which sits before the archive_checker.check() call, so the raise itself diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index 74c6f7e5c..898d1e725 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -1,4 +1,5 @@ import hashlib +import logging import os import time from datetime import UTC, datetime @@ -32,7 +33,6 @@ from ..helpers import CorruptPack, Error, bin_to_hex, safe_ns from ..helpers.msgpack import int_to_timestamp from ..manifest import Manifest from ..repository import PackReader, Repository -from .repository_test import Interrupter class TestAdHocWithFilesCache: @@ -567,26 +567,31 @@ def test_build_chunkindex_drops_a_pack_that_validates_nothing_when_others_do(tmp def two_pack_repo(tmp_path): - """A repository with two single-object packs, so a rebuild has a pack boundary to stop at.""" + """A repository with two packs of two objects each: H(90) and H(92) in one, H(91) and H(93) in the other.""" from .repository_test import fchunk repository = Repository(os.fspath(tmp_path / "repository"), exclusive=True, create=True) with repository: - repository.store_store("packs/" + bin_to_hex(H(95)), fchunk(b"one", chunk_id=H(90))) - repository.store_store("packs/" + bin_to_hex(H(96)), fchunk(b"two", chunk_id=H(91))) + pack1 = fchunk(b"one", chunk_id=H(90)) + fchunk(b"three", chunk_id=H(92)) + pack2 = fchunk(b"two", chunk_id=H(91)) + fchunk(b"four", chunk_id=H(93)) + repository.store_store("packs/" + bin_to_hex(H(95)), pack1) + repository.store_store("packs/" + bin_to_hex(H(96)), pack2) return repository -def read_first_pack_then_interrupt(monkeypatch, interrupter): - """Trip interrupter once a pack has been read; return the list the walk appends each pack id to.""" +def interrupt_after(monkeypatch, interrupter, objects): + """Trip interrupter once the walk yielded that many objects of a pack, or at the end of the pack if + objects is None. Return the list the walk appends each pack id to.""" packs_read = [] orig_iter_headers = PackReader.iter_headers def iter_headers(self, **kwargs): packs_read.append(self.pack_id) - headers = list(orig_iter_headers(self, **kwargs)) - interrupter.triggered = True # one Ctrl-C, after the first pack was indexed - return iter(headers) + for n, header in enumerate(orig_iter_headers(self, **kwargs)): + if n == objects: + interrupter.triggered = True + yield header + interrupter.triggered = True monkeypatch.setattr(PackReader, "iter_headers", iter_headers) return packs_read @@ -595,26 +600,39 @@ def read_first_pack_then_interrupt(monkeypatch, interrupter): def test_build_chunkindex_slow_rebuild_ignores_sigint_by_default(tmp_path, monkeypatch): """Without interruptible, the rebuild walks every pack and returns a complete index, also after a Ctrl-C. ArchiveChecker.finish() relies on that, #9850.""" + from .repository_test import Interrupter + interrupter = Interrupter() monkeypatch.setattr(cache_mod, "sig_int", interrupter) - packs_read = read_first_pack_then_interrupt(monkeypatch, interrupter) + packs_read = interrupt_after(monkeypatch, interrupter, objects=1) with two_pack_repo(tmp_path) as repository: index = build_chunkindex_from_repo(repository, slow_rebuild=True) assert len(packs_read) == 2 # the interrupt did not stop the walk - assert H(90) in index and H(91) in index + assert all(H(i) in index for i in (90, 91, 92, 93)) -def test_build_chunkindex_slow_rebuild_interruptible_discards_the_partial_index(tmp_path, monkeypatch): - """With interruptible, a Ctrl-C ends the rebuild at the pack boundary and raises; the index built so - far reaches neither the caller nor the repo, #10042.""" +@pytest.mark.parametrize( + "objects, packs_done", + [(None, 1), (1, 0)], # a Ctrl-C at the end of the first pack, or after its first object + ids=["between-packs", "within-pack"], +) +def test_build_chunkindex_slow_rebuild_interruptible_discards_the_partial_index( + tmp_path, monkeypatch, caplog, objects, packs_done +): + """With interruptible, a Ctrl-C stops the rebuild before the next object and raises; the index built + so far reaches neither the caller nor the repo, #10042.""" + from .repository_test import Interrupter + interrupter = Interrupter() monkeypatch.setattr(cache_mod, "sig_int", interrupter) - packs_read = read_first_pack_then_interrupt(monkeypatch, interrupter) + packs_read = interrupt_after(monkeypatch, interrupter, objects=objects) with two_pack_repo(tmp_path) as repository: stored_before = set(list_chunkindex_hashes(repository)) - with pytest.raises(ChunkIndexRebuildInterrupted, match="Got Ctrl-C"): - build_chunkindex_from_repo(repository, slow_rebuild=True, write_immediately=True, interruptible=True) - assert len(packs_read) == 1 # stopped at the boundary, the second pack was not read + with caplog.at_level(logging.INFO, logger="borg.cache"): + with pytest.raises(ChunkIndexRebuildInterrupted, match="Got Ctrl-C"): + build_chunkindex_from_repo(repository, slow_rebuild=True, write_immediately=True, interruptible=True) + assert f"Chunk index rebuild interrupted after {packs_done} of 2 packs." in caplog.text + assert len(packs_read) == 1 # the second pack was not read assert set(list_chunkindex_hashes(repository)) == stored_before # nothing was stored diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index ffb053072..0ac592a37 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -1535,9 +1535,16 @@ def test_check_repair_leaves_index_when_interrupted(tmp_path, caplog, monkeypatc assert repository.check(repair=False) is False # repair left the index corrupt -def test_check_repair_index_rebuild_interrupted(tmp_path, caplog, monkeypatch): +def reject_any(chunk_id, obj): + # A validator that rejects every object, so the index rebuild skips (drops) every object it reads. + return False + + +@pytest.mark.parametrize("validate", [validate_any, reject_any], ids=["accept", "reject"]) +def test_check_repair_index_rebuild_interrupted(tmp_path, caplog, monkeypatch, validate): # a Ctrl-C after the packs were verified, while the repair rebuilds the index from them: the rebuild - # ends at the pack boundary, the corrupt fragments stay, and the check reports the index as corrupt. + # stops, the corrupt fragments stay, and the check reports the index as corrupt, also when the + # rebuild skipped objects before the Ctrl-C. from .. import cache as cache_module location = os.fspath(tmp_path / "repo") @@ -1558,9 +1565,8 @@ def test_check_repair_index_rebuild_interrupted(tmp_path, caplog, monkeypatch): def iter_headers_then_interrupt(self, **kwargs): packs_read.append(self.pack_id) - headers = list(orig_iter_headers(self, **kwargs)) - interrupter.triggered = True # one Ctrl-C after the rebuild indexed the first pack - return iter(headers) + yield from orig_iter_headers(self, **kwargs) + interrupter.triggered = True # one Ctrl-C after the rebuild walked the first pack # check() and the rebuild each read sig_int through their own module namespace. monkeypatch.setattr(repository_module, "sig_int", interrupter) @@ -1570,9 +1576,10 @@ def test_check_repair_index_rebuild_interrupted(tmp_path, caplog, monkeypatch): assert len(repository.store_list("packs")) > 1 # there is a pack boundary to stop at index_before = set(info.name for info in repository.store_list("index")) with caplog.at_level(logging.WARNING, logger="borg.repository"): - assert repository.check(repair=True, validate=validate_any) is False + assert repository.check(repair=True, validate=validate) is False assert "Index rebuild interrupted" in caplog.text assert "Interrupted full repository check, index still corrupt so far." in caplog.text + assert "index rebuilt" not in caplog.text assert len(packs_read) == 1 # the rebuild stopped at the pack boundary assert set(info.name for info in repository.store_list("index")) == index_before # nothing stored monkeypatch.setattr(PackReader, "iter_headers", orig_iter_headers)