diff --git a/src/borg/archiver/compact_cmd.py b/src/borg/archiver/compact_cmd.py index a7795b8a0..a993ccf08 100644 --- a/src/borg/archiver/compact_cmd.py +++ b/src/borg/archiver/compact_cmd.py @@ -508,6 +508,8 @@ class CompactMixIn: ``borg compact`` does not rewrite or merge packs that ``borg check`` recorded as corrupt and warns about them. ``borg check --repair --verify-data`` deletes the corrupt chunks. + That repair does not remove damage outside any chunk (e.g. bytes appended to a pack), so such a + pack stays recorded corrupt and ``borg compact`` warns about it on every run (refs #10026). You usually do not want to run ``borg compact`` after every write operation, but either regularly (e.g., once a month, possibly together with ``borg check``) or diff --git a/src/borg/archiver/repo_compress_cmd.py b/src/borg/archiver/repo_compress_cmd.py index 66f759c92..f377ca8c3 100644 --- a/src/borg/archiver/repo_compress_cmd.py +++ b/src/borg/archiver/repo_compress_cmd.py @@ -7,11 +7,11 @@ from ._common import with_repository, Highlander from ..cache import build_chunkindex_from_repo, delete_chunkindex_from_repo, write_chunkindex_to_repo from ..compress import ObfuscateSize, Auto, COMPRESSOR_TABLE from ..constants import * # NOQA -from ..helpers import sig_int, ProgressIndicatorPercent, Error, CompressionSpec -from ..helpers import format_file_size, hex_to_bin +from ..helpers import sig_int, ProgressIndicatorPercent, Error, CompressionSpec, set_ec, EXIT_WARNING +from ..helpers import format_file_size, bin_to_hex, hex_to_bin from ..helpers.argparsing import ArgumentParser from ..repoobj import object_validator -from ..repository import Repository +from ..repository import Repository, PackTracker from ..logger import create_logger @@ -56,6 +56,9 @@ class PackRecompressor: Each pack is loaded once (via Repository.transform_pack); objects not stored with the desired compression get recompressed, the others are carried into the rewritten pack unchanged. A pack whose objects all already match is not rewritten at all. + + A pack recorded corrupt in PackTracker is not rewritten: a pack's id is the hash of its content, + so a rewritten copy of the corrupt bytes would get a new id that passes "borg check". """ def __init__(self, repository, manifest, *, print_stats): @@ -91,7 +94,8 @@ class PackRecompressor: size_before = sum(size for _, size in packs) size_after = size_before - stale_packs = set(per_pack) - {pack_id for pack_id, _ in packs} + present_packs = {pack_id for pack_id, _ in packs} + stale_packs = set(per_pack) - present_packs if stale_packs: # stale entries reference a pack absent from the store. they are kept, borg check --repair removes them. stale = sum(len(per_pack[pack_id]) for pack_id in stale_packs) @@ -99,6 +103,17 @@ class PackRecompressor: f'index entries referencing a missing pack file: {stale}. Run "borg check --repair" to remove them.' ) + # packs recorded corrupt in PackTracker that are still in the store + corrupt_packs = set(PackTracker.load(self.repository).corrupt_ids()) & present_packs + if corrupt_packs: + logger.warning( + f'{len(corrupt_packs)} pack(s) recorded corrupt by "borg check" are not rewritten. ' + 'Run "borg check --repair --verify-data".' + ) + for pack_id in sorted(corrupt_packs): + logger.debug(f"Corrupt pack: {bin_to_hex(pack_id)}") + set_ec(EXIT_WARNING) + pi = ProgressIndicatorPercent( total=len(packs), msg="Recompressing %3.1f%%", step=0.1, msgid="repo_compress.recompress" ) @@ -108,7 +123,7 @@ class PackRecompressor: break # stop cleanly at a pack boundary: save the index below, then raise ids = per_pack.get(pack_id) # a pack without indexed objects (all-gap) is left for "borg check --repair", see #9868. - if ids: + if ids and pack_id not in corrupt_packs: new_pack_id, new_size = self.repository.transform_pack( pack_id, ids, @@ -214,6 +229,11 @@ class RepoCompressMixIn: desired compression type/level - if no compression gives a shorter output, that might be chosen; such chunks are kept as they are. + ``borg repo-compress`` does not rewrite packs that ``borg check`` recorded as corrupt + and warns about them. ``borg check --repair --verify-data`` deletes the corrupt chunks. + That repair does not remove damage outside any chunk (e.g. bytes appended to a pack), so such a + pack stays recorded corrupt and is not rewritten (refs #10026). + Rewriting a pack invalidates every client's cached chunk index, so the next borg operation of each client will re-fetch the chunk index from the repository. diff --git a/src/borg/testsuite/archiver/repo_compress_cmd_test.py b/src/borg/testsuite/archiver/repo_compress_cmd_test.py index 140473303..883865533 100644 --- a/src/borg/testsuite/archiver/repo_compress_cmd_test.py +++ b/src/borg/testsuite/archiver/repo_compress_cmd_test.py @@ -13,7 +13,7 @@ from ...archiver.repo_compress_cmd import PackRecompressor from .. import make_test_key from . import create_regular_file, cmd, open_repository, RK_ENCRYPTION -from ..repository_test import H, accept_all, fchunk, pdchunk +from ..repository_test import H, accept_all, fchunk, pdchunk, corrupt_chunk_on_disk def test_repo_compress(archiver): @@ -240,6 +240,39 @@ def test_repo_compress_soft_interrupt_persists_valid_index(archiver, monkeypatch cmd(archiver, "check") +@pytest.mark.parametrize("corrupt_ctype", (CNONE.ID, ZLIB.ID)) +def test_repo_compress_keeps_corrupt_pack(archiver, corrupt_ctype): + # repo-compress keeps a pack "borg check" recorded corrupt unchanged and warns (#10410). + # with "-C none", the corrupt object is either already stored as wanted (CNONE) or needs recompressing (ZLIB). + create_regular_file(archiver.input_path, "rand", contents=os.urandom(100_000)) # stored uncompressed + create_regular_file(archiver.input_path, "text", contents=b"z" * 100_000) # stored zlib compressed + cmd(archiver, "repo-create", RK_ENCRYPTION) + cmd(archiver, "create", "test", "input", "-C", "zlib,3") + with open_repository(archiver) as repository: + manifest = Manifest.load(repository) + ctypes = {} # pack_id -> {ctype, ...} + for id, entry in repository.chunks.iteritems(): + meta = manifest.repo_objs.parse_meta(id, repository.get(id, read_data=False), ro_type=ROBJ_DONTCARE) + ctypes.setdefault(entry.pack_id, set()).add(meta["ctype"]) + if meta["ctype"] == corrupt_ctype and meta["size"] == 100_000: + corrupt_id = id + corrupt_pack = repository.chunks[corrupt_id].pack_id + # the corrupt pack holds both objects, and another pack holds an object "-C none" recompresses. + assert ctypes.pop(corrupt_pack) >= {CNONE.ID, ZLIB.ID} + assert any(ZLIB.ID in pack_ctypes for pack_ctypes in ctypes.values()) + packs_before = {info.name for info in repository.store_list("packs")} + corrupt_chunk_on_disk(repository, corrupt_id) # corrupts the data, the metadata stays readable + cmd(archiver, "check", exit_code=1) + + output = cmd(archiver, "repo-compress", "-v", "-C", "none", exit_code=EXIT_WARNING) + assert '1 pack(s) recorded corrupt by "borg check" are not rewritten.' in output + with open_repository(archiver) as repository: + packs_after = {info.name for info in repository.store_list("packs")} + assert bin_to_hex(corrupt_pack) in packs_after + assert len(packs_before - packs_after) >= 1 # other packs were recompressed + cmd(archiver, "check", exit_code=1) + + def transform_via(replacements): """Make a transform_pack callback replacing the given objects, keeping all others unchanged."""