From c5b4ea510fb14f28192ef5630100db11ec1ef23e Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 7 Oct 2026 00:05:46 +0530 Subject: [PATCH 1/5] repository: split CheckTracker out of PackTracker, refs #10025 --- src/borg/repository.py | 72 +++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 29 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index e3c19c40dc..772f4790aa 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -730,21 +730,19 @@ def remove_missing_pack_entries(chunks, missing_pack_ids): SalvageResult = namedtuple("SalvageResult", "status new_pack_id kept dropped_bytes removed_ids") -class PackTracker: - """Pack verification results, mapping pack_id -> (timestamp, result). +class CheckTracker: + """Check results, mapping a 32-byte id -> (timestamp, result). - Records are kept across checks: intact records (result=1) are reused by checks run with - max_age, corrupt records (result=0) are kept for repair and always re-verified. Records of - packs no longer listed in packs/ are pruned when a check finishes scanning packs/. - Stored at cache/checked-packs as the serialized table in the repository key's envelope (see + Stored at NAME as the serialized table in the repository key's envelope (see Repository.store_encrypt_store). new() starts an empty tracker, load() reads the stored one. + Subclasses set NAME. """ - NAME = "cache/checked-packs" - KEY_SIZE = 32 # pack id + NAME: str + KEY_SIZE = 32 Entry = namedtuple("Entry", "timestamp result") EntryFormatT = namedtuple("EntryFormatT", "timestamp result") - _EntryFormat = EntryFormatT(timestamp="Q", result="B") # unix ts, 1=ok 0=corrupt + _EntryFormat = EntryFormatT(timestamp="Q", result="B") # unix ts, 1=ok 0=failed def __init__(self, repository, table): self.repository = repository @@ -760,21 +758,22 @@ def new(cls, repository): def load(cls, repository): """Return a tracker holding the stored table. - Return an empty one if cache/checked-packs is missing, fails the authentication of the key's + Return an empty one if the object at NAME is missing, fails the authentication of the key's envelope, does not deserialize, or its entries do not have this class's key size and Entry layout. """ + label = cls.NAME.rpartition("/")[2] try: data = repository.store_load_decrypt(cls.NAME) except StoreObjectNotFound: return cls.new(repository) except IntegrityError: - logger.warning("Ignoring corrupted checked-packs set.") + logger.warning(f"Ignoring corrupted {label} set.") return cls.new(repository) try: with io.BytesIO(data) as f: table = HashTableNT.read(f) except ValueError: - logger.warning("Ignoring unreadable checked-packs set.") + logger.warning(f"Ignoring unreadable {label} set.") return cls.new(repository) # read() takes key size and value type from the blob itself, so the table needs a layout check # against Entry here. All entries in a table share one layout, so checking one entry suffices. @@ -782,35 +781,35 @@ def load(cls, repository): if sample is not None: key, value = sample if len(key) != cls.KEY_SIZE or value._fields != cls.Entry._fields: - logger.warning("Ignoring checked-packs set with an unexpected layout.") + logger.warning(f"Ignoring {label} set with an unexpected layout.") return cls.new(repository) return cls(repository, table) def __len__(self): return len(self.table) - def get(self, pack_id): - """Return the Entry for pack_id, or None if it is not recorded.""" - return self.table.get(pack_id) + def get(self, id): + """Return the Entry for id, or None if it is not recorded.""" + return self.table.get(id) - def record(self, pack_id, ok): - self.table[pack_id] = self.Entry(timestamp=int(time.time()), result=int(ok)) + def record(self, id, ok): + self.table[id] = self.Entry(timestamp=int(time.time()), result=int(ok)) - def forget(self, pack_id): - """Drop the record of pack_id, if any.""" - self.table.pop(pack_id, None) + def forget(self, id): + """Drop the record of id, if any.""" + self.table.pop(id, None) - def corrupt_ids(self): - """Return the ids of the packs recorded corrupt, sorted.""" - return sorted(pack_id for pack_id, entry in self.table.items() if not entry.result) + def failed_ids(self): + """Return the ids recorded with result=0, sorted.""" + return sorted(id for id, entry in self.table.items() if not entry.result) - def prune(self, pack_ids): - """Drop the records whose pack id is not in pack_ids (the set of pack ids listed in packs/), - then store the remaining records (or delete the stored object if none remain). + def prune(self, ids): + """Drop the records whose id is not in ids, then store the remaining records (or delete the + stored object if none remain). """ # the keys are collected first because the table must not be mutated while iterating it. - for pack_id in [pack_id for pack_id, _ in self.table.items() if pack_id not in pack_ids]: - del self.table[pack_id] + for id in [id for id, _ in self.table.items() if id not in ids]: + del self.table[id] if len(self.table): self.save() else: @@ -830,6 +829,21 @@ def clear(self): pass +class PackTracker(CheckTracker): + """Pack verification results, mapping pack_id -> (timestamp, result). + + Records are kept across checks: intact records (result=1) are reused by checks run with + max_age, corrupt records (result=0) are kept for repair and always re-verified. Records of + packs no longer listed in packs/ are pruned when a check finishes scanning packs/. + """ + + NAME = "cache/checked-packs" + + def corrupt_ids(self): + """Return the ids of the packs recorded corrupt, sorted.""" + return self.failed_ids() + + class _ConfigMissing(Exception): """internal: the store exists, but has no repository config object (see Repository._load_config).""" From f0de2d78513a28c11a7d21cdb644f8b3a28718e0 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 7 Oct 2026 00:21:28 +0530 Subject: [PATCH 2/5] check: --max-age and --max-duration cover the archives check, fixes #10025 Archive check results are kept in cache/checked-archives, refs #10356. --- docs/internals/data-structures.rst | 14 +- docs/internals/security.rst | 5 +- src/borg/archive.py | 114 ++++++++- src/borg/archiver/check_cmd.py | 106 ++++---- src/borg/repository.py | 60 ++++- src/borg/testsuite/archiver/check_cmd_test.py | 232 +++++++++++++++++- .../archiver/restricted_permissions_test.py | 4 + .../testsuite/archiver/store_objects_test.py | 5 + src/borg/testsuite/repository_test.py | 49 +++- 9 files changed, 514 insertions(+), 75 deletions(-) diff --git a/docs/internals/data-structures.rst b/docs/internals/data-structures.rst index f692f8c6fc..683815437a 100644 --- a/docs/internals/data-structures.rst +++ b/docs/internals/data-structures.rst @@ -80,6 +80,18 @@ cache/ ``check --repair`` salvages each pack still recorded corrupt (see :ref:`packs`) and records a pack that reads intact at the salvage as intact. Records of packs no longer listed in packs/ are pruned when a check finishes. + checked-archives + archives check results (archive id -> timestamp, result), in the same format as + ``checked-packs``. A result is ok if the check could read all metadata of the archive + and found every chunk it references in the chunks index. ``check --max-age`` skips + archives whose ok record is younger than the given age, and partial checks + (``--max-duration``) check the archives without an ok record first, then the + least-recently-checked ones. ``check --repair`` and ``check --verify-data`` check every + archive. All records are removed when a check finds a corrupt or missing pack or a + defect chunk, when it salvages a pack, and when the archives check starts with + ``--repair``. No records are stored while a pack is recorded corrupt in + ``checked-packs``. Records of archives no longer in archives/ (soft-deleted ones + included) are pruned when an archives check finishes. referenced-by-archive. what one archive references (object ID -> plaintext object size), plus the file count and content size of that archive, in the key's store object envelope (see @@ -114,7 +126,7 @@ locks/ .. _store_object_envelope: -The index fragments, the lock objects, ``checked-packs``, the +The index fragments, the lock objects, ``checked-packs``, ``checked-archives``, the ``referenced-by-archive.*`` objects and ``config/defaults`` are stored in the **store object envelope**: the repository key's ``encrypt()``, exactly as for the metadata and data slots of the objects in a pack (see :ref:`security_encryption`), with an empty id and an AAD of diff --git a/docs/internals/security.rst b/docs/internals/security.rst index 893b991a71..329ee5d3b9 100644 --- a/docs/internals/security.rst +++ b/docs/internals/security.rst @@ -389,8 +389,9 @@ used: client never ends up using key material of the attacker's choice. - ``keys/`` -- in ``repokey`` mode, the borg key(s), encrypted with the passphrase-derived KEK (see :ref:`key_encryption`). -- ``cache/*``: ``cache/checked-packs`` (the ``borg check`` results per pack) and the - per-archive reference caches ``cache/referenced-by-archive.`` +- ``cache/*``: ``cache/checked-packs`` and ``cache/checked-archives`` (the ``borg check`` + results per pack and per archive) and the per-archive reference caches + ``cache/referenced-by-archive.`` (written by ``borg compact`` and ``borg analyze``, they list the object ids and plaintext sizes an archive references) are in the key's store object envelope, like the index. The envelope binds each object to its repository and name, so the store diff --git a/src/borg/archive.py b/src/borg/archive.py index e5884efc62..69e7553923 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -54,7 +54,7 @@ from .item import Item, ArchiveItem, ItemDiff from .platform import acl_get, acl_set, set_flags, get_flags, set_times, swidth from .hashindex import ChunkIndex, ChunkIndexEntry -from .repository import Repository, PackReader, remove_missing_pack_entries +from .repository import Repository, PackReader, ArchiveTracker, PackTracker, remove_missing_pack_entries from .repoobj import RepoObj, object_validator # macOS: SF_DATALESS marks dataless placeholder files (e.g. cloud files not materialized locally). @@ -2349,6 +2349,12 @@ def __init__(self): # ids of the objects with ro_type ROBJ_ARCHIVE_META that verify_data found. # None if verify_data did not run or was interrupted. self.archive_meta_ids = None + # the archive check results, see ArchiveTracker. None until check() loads them. + self.tracker = None + # False if this run stores no archive check results, see check(). + self.keep_results = True + # True if rebuild_archives stopped at the deadline before it reached every archive. + self.stopped_at_deadline = False def record_stored(self, results): """Add the pack ids in results to written_packs. @@ -2392,6 +2398,8 @@ def check( newer=None, oldest=None, newest=None, + max_age=0, + deadline=None, ): """Perform a set of checks on 'repository' @@ -2403,6 +2411,20 @@ def check( :param oldest/newest: only check archives older/newer than timedelta from oldest/newest archive timestamp :param verify_data: integrity verification of data referenced by archives :param format: format string used to describe an archive in the log output + :param max_age: seconds, 0 = check every archive. Skip the archives whose ok record (see + ArchiveTracker) is younger than max_age. Ignored with repair or verify_data, and if this run + stores no archive check results (see below). + :param deadline: time.monotonic() value at which to stop, None = no limit. The check then + analyzes the archives without an ok record first, then the least recently checked ones, and + stops between two archives. It analyzes at least one archive, as the repository check + verifies at least one pack, so that every run makes progress. + + Each analyzed archive is recorded in cache/checked-archives. repair clears the records first, + because it rebuilds the index and rewrites the archives. A run clears the records and stores + none if a pack is recorded corrupt (see PackTracker), or if it runs without repair and has a + finding before it analyzes the archives (a missing pack, content the index rebuild dropped, a + verify_data or find_lost_archives error): the chunk index may name chunks that are lost or + defect then. """ if not isinstance(repository, Repository): logger.error("Checking legacy repositories is not supported.") @@ -2413,6 +2435,9 @@ def check( self.verifying_data = verify_data self.format = format self.repository = repository + self.tracker = ArchiveTracker.load(repository) + if repair: + self.tracker.clear() # A normal (non-repair) archives check trusts the in-repo index: the repository check verified # each index object's store hash, and the index is the authoritative record of which chunks exist, # so we do not rebuild it from the packs (reading every pack is far too slow for a routine check). @@ -2474,6 +2499,11 @@ def check( # On Ctrl-C, skip any scan not yet started; a scan already running stops at its own boundary. if find_lost_archives and not sig_int: self.rebuild_archives_directory() + if (self.error_found and not repair) or PackTracker.load(repository).corrupt_ids(): + self.keep_results = False + self.tracker.clear() + if repair or verify_data or not self.keep_results: + max_age = 0 if not sig_int: self.rebuild_archives( match=match, @@ -2484,6 +2514,8 @@ def check( oldest=oldest, newer=newer, newest=newest, + max_age=max_age, + deadline=deadline, ) # finish() writes a consistent chunk index; run it on Ctrl-C too (#9850). self.finish() @@ -2493,7 +2525,12 @@ def check( else: logger.info("Archive consistency check interrupted, no problems found so far.") raise Error("Got Ctrl-C / SIGINT.") - if self.error_found: + if self.stopped_at_deadline: + if self.error_found: + logger.error("Archive consistency check stopped by --max-duration, problems found so far.") + else: + logger.info("Archive consistency check stopped by --max-duration, no problems found so far.") + elif self.error_found: logger.error("Archive consistency check complete, problems found.") else: logger.info("Archive consistency check complete, no problems found.") @@ -2822,9 +2859,37 @@ def check_archive_meta(chunk_id): logger.info("Rebuilding missing archives directory entries completed.") def rebuild_archives( - self, first=0, last=0, sort_by="", match=None, older=None, newer=None, oldest=None, newest=None + self, + first=0, + last=0, + sort_by="", + match=None, + older=None, + newer=None, + oldest=None, + newest=None, + max_age=0, + deadline=None, ): - """Analyze and rebuild archives, expecting some damage and trying to make stuff consistent again.""" + """Analyze and rebuild archives, expecting some damage and trying to make stuff consistent again. + + max_age, deadline: see check(). + """ + tracker = self.tracker + analyzed = 0 # archives analyzed in this run + reused = 0 # archives skipped because of a recent ok record + + def record_result(archive_id, found_before, *, complete=True): + """Record the result of the archive just analyzed, then merge it into error_found. + + error_found holds the findings of this archive only while it is analyzed: the loop below + resets it for each archive and passes its previous value as found_before. + An archive that was not analyzed to its end (complete=False) gets a record only if it has a + finding. + """ + if complete or self.error_found: + tracker.record(archive_id, not self.error_found) + self.error_found = self.error_found or found_before # Missing file chunks, collected during the per-archive checks and reported grouped as # chunk -> files -> archives after all archives were analyzed. Bounded by @@ -3037,6 +3102,16 @@ def robust_item_ids(): else: archive_infos = self.manifest.archives.list(sort_by=sort_by) num_archives = len(archive_infos) + if deadline is not None: + # archives without an ok record first, then the least recently checked ones, so that + # repeated runs reach every archive. The sort is stable, so sort_by orders equal keys. + def check_order(info): + entry = tracker.get(info.id) + if entry is None: + return False, 0 + return bool(entry.result), entry.timestamp + + archive_infos.sort(key=check_order) formatter = ArchiveFormatter(self.format, self.repository, self.manifest, self.key) pi = ProgressIndicatorPercent( @@ -3050,8 +3125,16 @@ def robust_item_ids(): if sig_int: # --repair rewrites each archive as a whole, so with --repair the check stops only here. break + if deadline is not None and analyzed and time.monotonic() >= deadline: + self.stopped_at_deadline = True + break pi.show(i) archive_id, archive_id_hex = info.id, bin_to_hex(info.id) + if tracker.is_recent(archive_id, max_age): + reused += 1 + continue + analyzed += 1 + found_before, self.error_found = self.error_found, False try: formatted = formatter.format_item(info, jsonline=False) # the formatter uses defaults for keys like {comment} if it has no archive metadata. @@ -3071,6 +3154,7 @@ def robust_item_ids(): self.manifest.archives.delete_by_id(archive_id) else: logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") + record_result(archive_id, found_before) continue cdata = self.repository.get(archive_id) try: @@ -3083,6 +3167,7 @@ def robust_item_ids(): self.manifest.archives.delete_by_id(archive_id) else: logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") + record_result(archive_id, found_before) continue archive = self.key.unpack_archive(data) archive = ArchiveItem(internal_dict=archive) @@ -3090,9 +3175,11 @@ def robust_item_ids(): raise Exception("Unknown archive metadata version") items_buffer = ChunkBuffer(self.key) items_buffer.write_chunk = add_callback + complete = True 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. + complete = False break if "chunks" in item: verify_file_chunks(info.name, item) @@ -3111,9 +3198,28 @@ def robust_item_ids(): self.create_archive_entry(info.name, new_archive_id, info.ts) if archive_id != new_archive_id: self.manifest.archives.delete_by_id(archive_id) + archive_id = new_archive_id + record_result(archive_id, found_before, complete=complete) finally: pi.finish() report_missing_chunks() + if self.stopped_at_deadline: + # like a pack recorded corrupt, an archive recorded with a problem fails the check until + # a check finds it ok again. + unchecked = archive_infos[analyzed + reused :] + failed_ids = set(tracker.failed_ids()) + failed = sum(1 for info in unchecked if info.id in failed_ids) + if failed: + self.error_found = True + logger.error(f"{failed} archive(s) recorded with problems were not checked again in this run.") + logger.info(f"Stopping the archive check after --max-duration, {len(unchecked)} archive(s) left.") + summary = f"Analyzed {analyzed} archive(s)." + if reused: + summary += f" Reused {reused} recent archive check result(s)." + logger.info(summary) + if self.keep_results: + archives = self.manifest.archives + tracker.prune(set(archives.ids()) | set(archives.ids(deleted=True))) def verify_written_packs(self): """Read the object headers of the packs in written_packs and make the chunks index match them. diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index 21e6f692e2..3196499c1b 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -1,4 +1,5 @@ import os +import time from ._common import with_repository, Highlander from ..archive import ArchiveChecker @@ -88,17 +89,15 @@ def do_check(self, args, repository): max_age = 0 if args.repair and args.max_duration: raise CommandError("--repair does not allow --max-duration argument.") - if args.archives_only and args.max_age is not None: - # --max-age only affects the repository check; --archives-only skips it. - raise CommandError("--archives-only does not allow the --max-age option.") - if args.max_duration and not args.repo_only: - # --max-duration limits only the repository check; the archives check has no max_duration - # support. - raise CommandError("--repository-only is required for --max-duration support.") + if args.max_duration and (args.verify_data or args.find_lost_archives): + # both scan the whole repository and cannot be split into partial runs. + raise CommandError("--max-duration does not allow the --verify-data and --find-lost-archives options.") # every check needs the key, a --repository-only one included. ask NOW for the passphrase, not # after a repository check that can take hours, #1931. the key class comes from the repository # config, so loading the key reads no object. key = key_factory(repository) + # one time budget for the repository check and the archives check. + deadline = time.monotonic() + args.max_duration if args.max_duration else None if not args.repo_only: archive_checker = ArchiveChecker() archive_checker.key = key @@ -132,7 +131,12 @@ def do_check(self, args, repository): raise Error("Got Ctrl-C / SIGINT.") if not check_repository_defaults(repository, repair=args.repair): set_ec(EXIT_WARNING) - if not args.repo_only and not archive_checker.check( + if args.repo_only: + return + if deadline is not None and not args.archives_only and time.monotonic() >= deadline: + logger.info("Skipping the archive check: the repository check used up --max-duration.") + return + if not archive_checker.check( repository, verify_data=args.verify_data, repair=args.repair, @@ -146,9 +150,10 @@ def do_check(self, args, repository): oldest=args.oldest, newest=args.newest, format=format, + max_age=max_age, + deadline=deadline, ): set_ec(EXIT_WARNING) - return def build_parser_check(self, subparsers, common_parser, mid_common_parser): from ._common import process_epilog @@ -171,8 +176,7 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): A corrupt index ends the check after this step, as the archives check needs it, unless ``--repair`` is given (see below). This step also verifies the repository defaults object (see ``borg repo-create``): it must be present and authenticate - with the key. Running the repository check can - be split into multiple partial checks using ``--max-duration``. + with the key. For ssh:// repositories, the server computes the hashes, so the pack contents do not have to travel over the network. For other remote backends, borg usually has to read (download) the objects to hash them. @@ -204,42 +208,52 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): aborts if the key can not be loaded. The ``--max-age`` option makes the check reuse the results of previous - repository checks: packs whose intact result is younger than the given - timespan (e.g. ``--max-age=4w`` or ``--max-age=12m``) are skipped, spreading - the verification cost over repeated checks. The timespan uses the same markers - as ``--older``/``--newer``: ``d``, ``w``, ``H``, ``M``, ``S`` are exact spans, - while ``m`` and ``y`` are calendar units counted from now (so ``12m`` equals - ``1y``). Check results are recorded in any case; ``--max-age`` only controls - their reuse. Packs recorded corrupt are always re-verified. ``--max-age`` - affects only the repository check and cannot be combined with - ``--archives-only``. - - ``--repair`` reuses intact results in the same way. It always re-verifies the + checks: packs whose intact result and archives whose ok result is younger than + the given timespan (e.g. ``--max-age=4w`` or ``--max-age=12m``) are skipped, + spreading the verification cost over repeated checks. The timespan uses the same + markers as ``--older``/``--newer``: ``d``, ``w``, ``H``, ``M``, ``S`` are exact + spans, while ``m`` and ``y`` are calendar units counted from now (so ``12m`` + equals ``1y``). Check results are recorded in any case; ``--max-age`` only + controls their reuse. Packs recorded corrupt are always re-verified and + archives recorded with a problem are always checked again. + + An archive result is ok if all metadata of the archive could be read and all + chunks it references are present. The archive results are not reused with + ``--repair`` and ``--verify-data``. All of them are dropped when a check finds a + corrupt or missing pack or a defect chunk and when the archives check runs with + ``--repair``, because chunks the archives reference may be lost then. While a + pack is recorded corrupt, no archive results are stored. + + ``--repair`` reuses intact pack results in the same way. It always re-verifies the packs recorded corrupt, which are the ones it salvages. With ``--repository-only``, a corrupt repository index makes the repair verify every pack and ignore ``--max-age``, because it rebuilds the index from the packs it verified in that run. - The ``--max-duration`` option splits a long-running repository check into - several partial checks. After the given number of seconds, the check is - interrupted. A partial check verifies the least-recently-checked packs first, - so repeated runs cover the whole repository. Add ``--max-age`` to also skip - packs whose result is still younger than the given age: once every pack has a - recent result, further runs re-check each pack at most once per ``--max-age``, - and no faster than the per-run budget allows. + The ``--max-duration`` option splits a long-running check into several partial + checks. The given number of seconds is one time budget for the repository + check and the archives check: the repository check stops after the pack it is + verifying when the budget is used up, the archives check gets the time that is + left, checks at least one archive and stops between two archives. A partial + check verifies the least-recently-checked packs first, and it checks the + archives without an ok result first, then the least-recently-checked ones, so + repeated runs cover the whole repository. An archive recorded with a problem + that a partial archives check did not get to still fails that check. + + Add ``--max-age`` to also skip packs and archives whose result is still younger + than the given age: once every pack has a recent result, further runs re-check + each pack at most once per ``--max-age``, and no faster than the per-run budget + allows. Without ``--max-age``, a repository check that does not finish within + the budget uses it up in every run, so the archives check is skipped every + time; use ``--max-age``, or run ``--repository-only`` and ``--archives-only`` + checks with a budget each. Assuming a complete check would take 7 hours, running a daily check with - ``--max-duration=3600 --max-age=1w`` (1 hour) results in one full repository - verification per week. Partial repository checks run neither archive checks - nor repair mode, so ``--max-duration`` requires ``--repository-only`` and - cannot be combined with ``--archives-only`` or ``--repair``. - - **Note:** A partial repository check verifies the repository files in exactly the - same way as a full repository check does - the difference is only how many of them - one run gets to. What a partial run does not do are the archive checks: because - ``--max-duration`` requires ``--repository-only``, neither the archive metadata - checks nor the cryptographic data verification of ``--verify-data`` run. Partial - checks are therefore mostly useful for very large repositories where a full check - would take too long. + ``--max-duration=3600 --max-age=1w`` (1 hour) results in one full + verification per week. + + ``--max-duration`` cannot be combined with ``--repair``, ``--verify-data`` or + ``--find-lost-archives``: these work on the whole repository and cannot be + split into partial runs. The ``--verify-data`` option will perform a full integrity verification of data, which means reading the data from the repository, decrypting and decompressing it. @@ -275,9 +289,9 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): The repository check stops after the current pack; ``--verify-data`` and ``--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. + the interrupt are kept, so a later check does not re-verify those packs and archives 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 stops that rebuild after the current object and @@ -371,7 +385,7 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): type=relative_time_marker_validator, default=None, action=Highlander, - help="reuse intact-pack check results younger than TIMESPAN, e.g. 4w or 12m", + help="reuse the pack and archive check results younger than TIMESPAN, e.g. 4w or 12m", ) subparser.add_argument( "--max-duration", @@ -380,7 +394,7 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): type=int, default=0, action=Highlander, - help="perform only a partial repository check for at most SECONDS seconds (default: unlimited)", + help="perform only a partial check for at most SECONDS seconds (default: unlimited)", ) subparser.add_argument( "--format", diff --git a/src/borg/repository.py b/src/borg/repository.py index 772f4790aa..484729b12b 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -17,6 +17,7 @@ from borgstore.backends.errors import BackendConnectionError as StoreBackendConnectionError from borgstore.backends.errors import BackendDoesNotExist as StoreBackendDoesNotExist from borgstore.backends.errors import BackendAlreadyExists as StoreBackendAlreadyExists +from borgstore.backends.errors import PermissionDenied as StorePermissionDenied from .constants import * # NOQA from .hashindex import ChunkIndex, ChunkIndexEntry @@ -795,6 +796,18 @@ def get(self, id): def record(self, id, ok): self.table[id] = self.Entry(timestamp=int(time.time()), result=int(ok)) + def is_recent(self, id, max_age): + """Return True if id has an ok record younger than max_age seconds (0 = never). + + The timestamp is set by the client that ran the earlier check, so a future one (negative age) + is accepted up to MAX_CLOCK_SKEW. + """ + entry = self.table.get(id) + if entry is None or not entry.result or not max_age: + return False + age = time.time() - entry.timestamp + return -min(MAX_CLOCK_SKEW, max_age) <= age < max_age + def forget(self, id): """Drop the record of id, if any.""" self.table.pop(id, None) @@ -844,6 +857,34 @@ def corrupt_ids(self): return self.failed_ids() +class ArchiveTracker(CheckTracker): + """Archive check results, mapping archive_id -> (timestamp, result). + + result=1: the archives check read all metadata of the archive and found every chunk it references + in the chunk index. Archives checks run with max_age reuse such a record. result=0: the check found + a problem, such an archive is always checked again. + A check that finds a corrupt or missing pack or salvages a pack, and an archives check that + repairs, clear all records: chunks the archives reference may be gone then. An archives check + stores no records while a pack is recorded corrupt. Records of archives no longer in archives/ + (soft-deleted ones included) are pruned when an archives check finishes. + """ + + NAME = "cache/checked-archives" + + def save(self): + # a client without write permission for cache/ can still check archives, it just keeps no records. + try: + super().save() + except StorePermissionDenied: + logger.warning(f"Not storing the archive check results: no permission to write {self.NAME}.") + + def clear(self): + try: + super().clear() + except StorePermissionDenied: + logger.warning(f"Not clearing the archive check results: no permission to delete {self.NAME}.") + + class _ConfigMissing(Exception): """internal: the store exists, but has no repository config object (see Repository._load_config).""" @@ -1544,6 +1585,9 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=None, validat it. The record clears at the check that finds the pack intact again or gone (removed by compact, or replaced by a salvaged pack); prune() does this from packs/. + A run that finds a corrupt or missing pack, salvages a pack or skips a pack byte range in the + index rebuild clears cache/checked-archives (see ArchiveTracker), refs #10025. + It also reports missing packs (refs #9898): pack ids the chunk index references but that are absent from packs/. The index is read from its fragments only and its referenced pack ids are compared with the packs present in the store; store.info() confirms that each pack the listing @@ -1762,15 +1806,10 @@ def recorded_ts(info): self._lock_refresh() pack_pi.show(increase=1) # advance for skipped packs too, so the bar tracks packs/, not work done pack_id = hex_to_bin(info.name) - entry = tracker.get(pack_id) - # skip a pack recorded intact within the last max_age seconds. the timestamp is set - # by the client that ran the earlier check; accept a future one (negative age) up to - # MAX_CLOCK_SKEW, and re-verify anything at or past max_age. - if entry is not None and entry.result and max_age: - age = time.time() - entry.timestamp - if -min(MAX_CLOCK_SKEW, max_age) <= age < max_age: - pack_skipped += 1 - continue + # skip a pack recorded intact within the last max_age seconds. + if tracker.is_recent(pack_id, max_age): + pack_skipped += 1 + continue pack_files += 1 ok = verify("packs", info.name) if not ok: @@ -1843,6 +1882,9 @@ def note_drop(): # index_deferred: the archives phase rebuilds the corrupt index; it runs only if this check was not interrupted. index_deferred = bool(index_errors) and repair and not repo_only and not sig_int objs_errors = index_errors + pack_errors + len(missing_pack_ids) + drops + if pack_errors or missing_pack_ids or salvaged or drops: + # chunks the archives reference may be damaged or gone, so no archive check result holds. + ArchiveTracker.new(self).clear() summary = ( f"Checked {index_files} index files ({index_errors} errors) " f"and {pack_files} packs ({pack_errors} errors)." diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 0bc9930851..2623f7bcc0 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -4,6 +4,7 @@ import re import shutil import struct +import time import weakref from unittest.mock import patch @@ -11,6 +12,7 @@ from ...crypto.key import store_hash, STORE_HASH_NAME from ... import archive as archive_module +from ...archiver import check_cmd as check_cmd_module from ...archive import Archive, ArchiveChecker, ChunkBuffer from ...cache import ( Cache, @@ -31,7 +33,7 @@ from ...item import Item from ...manifest import Archives, Manifest from ...repoobj import RepoObj -from ...repository import PackReader, PackTracker, Repository +from ...repository import ArchiveTracker, PackReader, PackTracker, Repository from .. import changedir from ..repoobj_test import DATA_SIZE_OFFSET from ..repository_test import fchunk, corrupt_chunk_on_disk @@ -383,19 +385,18 @@ def test_check_max_age(archivers, request): archiver = request.getfixturevalue(archivers) check_cmd_setup(archiver) - # --archives-only does not allow --max-age; 0d is a valid value (resolves to no reuse). - # --max-duration needs --repository-only, but not --max-age: a partial check advances on its own. - if archiver.FORK_DEFAULT: - cmd(archiver, "check", "--archives-only", "--max-age=1d", exit_code=CommandError().exit_code) - cmd(archiver, "check", "--max-duration=3600", exit_code=CommandError().exit_code) - else: - with pytest.raises(CommandError): - cmd(archiver, "check", "--archives-only", "--max-age=1d") - with pytest.raises(CommandError): - cmd(archiver, "check", "--max-duration=3600") + # --max-duration does not allow the scans of the whole repository. + for option in "--verify-data", "--find-lost-archives": + if archiver.FORK_DEFAULT: + cmd(archiver, "check", "--max-duration=3600", option, exit_code=CommandError().exit_code) + else: + with pytest.raises(CommandError): + cmd(archiver, "check", "--max-duration=3600", option) - # a partial check runs without --max-age. + # a partial check runs without --max-age and without --repository-only. cmd(archiver, "check", "--repository-only", "--max-duration=3600", exit_code=0) + cmd(archiver, "check", "--max-duration=3600", exit_code=0) + cmd(archiver, "check", "--archives-only", "--max-duration=3600", "--max-age=0d", exit_code=0) # a check records its results, a later one with --max-age reuses them. output = cmd(archiver, "check", "-v", "--repository-only", exit_code=0) @@ -2521,3 +2522,210 @@ def test_items_with_unknown_keys_are_kept(archivers, request): assert items[0].as_dict()["newkey"] == "future" output = cmd(archiver, "check", "--archives-only", exit_code=0) assert "keys unknown to this borg version" in output # still just the warning + + +def archive_records(archiver): + """Return {archive name or id: result} of the records in cache/checked-archives. + + The key is the name for an archive in archives/, else the binary archive id. + """ + with open_repository(archiver) as repository: + manifest = Manifest.load(repository) + names = {info.id: info.name for info in manifest.archives.list()} + return {names.get(id, id): entry.result for id, entry in ArchiveTracker.load(repository).table.items()} + + +def test_check_archives_max_age(archivers, request): + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + assert archive_records(archiver) == {} + output = cmd(archiver, "check", "-v", "--archives-only", exit_code=0) + assert "Analyzed 2 archive(s)." in output + assert archive_records(archiver) == {"archive1": 1, "archive2": 1} + output = cmd(archiver, "check", "-v", "--archives-only", "--max-age=1d", exit_code=0) + assert "Analyzed 0 archive(s). Reused 2 recent archive check result(s)." in output + # a full check reuses them too. + output = cmd(archiver, "check", "-v", "--max-age=1d", exit_code=0) + assert "Analyzed 0 archive(s). Reused 2 recent archive check result(s)." in output + # an archive without a record is analyzed. + create_src_archive(archiver, "archive3") + output = cmd(archiver, "check", "-v", "--archives-only", "--max-age=1d", exit_code=0) + assert "Analyzed 1 archive(s). Reused 2 recent archive check result(s)." in output + assert archive_records(archiver) == {"archive1": 1, "archive2": 1, "archive3": 1} + # no reuse without --max-age, with --verify-data and with --repair. + for options in [], ["--verify-data", "--max-age=1d"], ["--repair", "--max-age=1d"]: + output = cmd(archiver, "check", "-v", "--archives-only", *options, exit_code=0) + assert "Analyzed 3 archive(s)." in output + assert "Reused" not in output + assert archive_records(archiver) == {"archive1": 1, "archive2": 1, "archive3": 1} + + +def test_check_archives_records_follow_the_archives_directory(archivers, request): + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + cmd(archiver, "check", "--archives-only", exit_code=0) + # the record of a soft-deleted archive is kept, so it is reused after an undelete. + cmd(archiver, "delete", "-a", "archive1") + output = cmd(archiver, "check", "-v", "--archives-only", "--max-age=1d", exit_code=0) + assert "Analyzed 0 archive(s). Reused 1 recent archive check result(s)." in output + cmd(archiver, "undelete", "-a", "archive1") + output = cmd(archiver, "check", "-v", "--archives-only", "--max-age=1d", exit_code=0) + assert "Analyzed 0 archive(s). Reused 2 recent archive check result(s)." in output + with open_repository(archiver) as repository: + old_ids = set(Manifest.load(repository).archives.ids()) + assert {id for id, _ in ArchiveTracker.load(repository).table.items()} == old_ids + # the repair rewrites the item metadata with the default ChunkBuffer size, so the archives get new + # ids, which it records. + cmd(archiver, "check", "--archives-only", "--repair", exit_code=0) + with open_repository(archiver) as repository: + new_ids = set(Manifest.load(repository).archives.ids()) + assert not new_ids & old_ids + assert {id for id, _ in ArchiveTracker.load(repository).table.items()} == new_ids + assert archive_records(archiver) == {"archive1": 1, "archive2": 1} + # compact removes the soft-deleted archives, the next check drops their records. + cmd(archiver, "delete", "-a", "archive1") + cmd(archiver, "compact") + cmd(archiver, "check", "--archives-only", "--max-age=1d", exit_code=0) + assert archive_records(archiver) == {"archive2": 1} + + +def test_check_archives_records_a_problem(archivers, request): + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + create_regular_file(archiver.input_path, "only_in_archive3", contents=b"only in archive3") + cmd(archiver, "create", "archive3", "input") + archive, repository = open_archive(archiver.repository_path, "archive3") + with repository: + item = next(item for item in archive.iter_items() if item.path.endswith("only_in_archive3")) + repository.delete(item.chunks[0].id, validate=None) + expected = {"archive1": 1, "archive2": 1, "archive3": 0} + output = cmd(archiver, "check", "-v", "--archives-only", exit_code=1) + assert "Analyzed 3 archive(s)." in output + assert archive_records(archiver) == expected + # the archive recorded with a problem is analyzed again. + output = cmd(archiver, "check", "-v", "--archives-only", "--max-age=1d", exit_code=1) + assert "Analyzed 1 archive(s). Reused 2 recent archive check result(s)." in output + assert f"Missing chunk detected: {bin_to_hex(item.chunks[0].id)}" in output + assert archive_records(archiver) == expected + + +@pytest.mark.parametrize("options", [["--archives-only"], []], ids=["archives-only", "full"]) +def test_check_archives_missing_pack_clears_the_records(archivers, request, monkeypatch, options): + archiver = request.getfixturevalue(archivers) + monkeypatch.setenv("BORG_PACK_MAX_COUNT", "1") # a pack per object: removing a pack removes one object + check_cmd_setup(archiver) + cmd(archiver, "check", "--archives-only", exit_code=0) + assert archive_records(archiver) == {"archive1": 1, "archive2": 1} + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + item = next(item for item in archive.iter_items() if item.path.endswith(src_file)) + chunk_id = item.chunks[-1].id + repository.store_delete("packs/" + bin_to_hex(repository.chunks[chunk_id].pack_id)) + # the ok records do not hide the chunk that is lost now. + output = cmd(archiver, "check", "-v", *options, "--max-age=1d", exit_code=1) + assert f"Missing chunk detected: {bin_to_hex(chunk_id)}" in output + assert "Analyzed 2 archive(s)." in output + assert "archive check result(s)" not in output + if options: + # the archives check found the missing pack itself, so it stores no result. + assert archive_records(archiver) == {} + else: + # the repository check removed the entries of the lost chunks from the index the archives check uses. + assert archive_records(archiver) == {"archive1": 0, "archive2": 0} + + +def test_check_archives_verify_data_error_clears_the_records(archivers, request): + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + cmd(archiver, "check", "--archives-only", exit_code=0) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + item = next(item for item in archive.iter_items() if item.path.endswith(src_file)) + corrupt_chunk_on_disk(repository, item.chunks[-1].id) + # the defect chunk is still in the index, so the archives pass and no result is stored. + cmd(archiver, "check", "--archives-only", "--verify-data", exit_code=1) + assert archive_records(archiver) == {} + + +def test_check_archives_stores_no_records_while_a_pack_is_recorded_corrupt(archivers, request): + archiver = request.getfixturevalue(archivers) + check_cmd_setup(archiver) + cmd(archiver, "check", exit_code=0) + assert archive_records(archiver) == {"archive1": 1, "archive2": 1} + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + item = next(item for item in archive.iter_items() if item.path.endswith(src_file)) + corrupt_chunk_on_disk(repository, item.chunks[-1].id) + # the repository check records the pack corrupt. the chunk is still in the index, so the archives pass. + output = cmd(archiver, "check", "-v", exit_code=1) + assert "Analyzed 2 archive(s)." in output + assert archive_records(archiver) == {} + output = cmd(archiver, "check", "-v", "--archives-only", "--max-age=1d", exit_code=0) + assert "Analyzed 2 archive(s)." in output + assert archive_records(archiver) == {} + + +class SteppingClock: + """Stands in for the time module: monotonic() returns 1, 2, 3, ...""" + + def __init__(self): + self.now = 0 + + def __getattr__(self, name): + return getattr(time, name) + + def monotonic(self): + self.now += 1 + return self.now + + +def test_check_archives_max_duration(archiver, monkeypatch): + check_cmd_setup(archiver) + check_args = dict(sort_by="ts", format="{archive}") + with KeyedRepository(archiver.repository_path, exclusive=True) as repository: + archives = Manifest.load(repository).archives + archive1_id, archive2_id = archives.get_one(["archive1"]).id, archives.get_one(["archive2"]).id + tracker = ArchiveTracker.new(repository) + tracker.table[archive2_id] = ArchiveTracker.Entry(timestamp=1000, result=1) + tracker.save() + + def run(deadline): + # rebuild_archives analyzes the first archive, then it reads the clock once before each + # further archive. + monkeypatch.setattr(archive_module, "time", SteppingClock()) + checker = ArchiveChecker() + ok = checker.check(repository, deadline=deadline, **check_args) + return ok, checker, ArchiveTracker.load(repository) + + # archive1 has no record, so it goes first. the deadline has passed, the check analyzes one + # archive anyway and stops before archive2. + ok, checker, tracker = run(0.5) + assert ok and checker.stopped_at_deadline + assert tracker.get(archive1_id).result == 1 + assert tuple(tracker.get(archive2_id)) == (1000, 1) + # an archive recorded with a problem fails a run that does not reach it. + tracker.forget(archive1_id) + tracker.table[archive2_id] = ArchiveTracker.Entry(timestamp=2**40, result=0) + tracker.save() + ok, checker, tracker = run(0.5) + assert not ok and checker.stopped_at_deadline + assert tracker.get(archive1_id).result == 1 + assert tuple(tracker.get(archive2_id)) == (2**40, 0) + # it goes before the archive recorded ok and is recorded ok again. + ok, checker, tracker = run(0.5) + assert ok and checker.stopped_at_deadline + assert tracker.get(archive2_id).result == 1 + # with time left, and without a deadline, every archive is analyzed. + for deadline in 1.5, None: + ok, checker, tracker = run(deadline) + assert ok and not checker.stopped_at_deadline + + +def test_check_max_duration_used_up_by_the_repository_check(archiver, monkeypatch): + check_cmd_setup(archiver) + # do_check reads the clock for the deadline (1 + 1) and again after the repository check (2). + monkeypatch.setattr(check_cmd_module, "time", SteppingClock()) + output = cmd(archiver, "check", "-v", "--max-duration=1", exit_code=0) + assert "Skipping the archive check: the repository check used up --max-duration." in output + assert "Starting archive consistency check" not in output + assert archive_records(archiver) == {} diff --git a/src/borg/testsuite/archiver/restricted_permissions_test.py b/src/borg/testsuite/archiver/restricted_permissions_test.py index 38bf1655f3..f9e77f95d8 100644 --- a/src/borg/testsuite/archiver/restricted_permissions_test.py +++ b/src/borg/testsuite/archiver/restricted_permissions_test.py @@ -125,6 +125,10 @@ def test_repository_permissions_read_only(archivers, request, monkeypatch): extracted_files = os.listdir("output") assert len(extracted_files) > 0 + # Check the archives: this works, but the check results cannot be stored. + output = cmd(archiver, "check", "--archives-only", exit_code=0) + assert "Not storing the archive check results" in output + # Try to create a new archive, which should fail. with pytest.raises(PermissionDenied): cmd(archiver, "create", "archive3", "input") diff --git a/src/borg/testsuite/archiver/store_objects_test.py b/src/borg/testsuite/archiver/store_objects_test.py index 928305858d..06c5c8235a 100644 --- a/src/borg/testsuite/archiver/store_objects_test.py +++ b/src/borg/testsuite/archiver/store_objects_test.py @@ -35,11 +35,14 @@ def test_index_and_cache_in_the_key_envelope(archivers, request, encryption): with open_repository(archiver) as repository: ids = [id for id, _ in repository.chunks.iteritems()] pack_ids = [hex_to_bin(info.name) for info in repository.store_list("packs")] + archive_ids = [hex_to_bin(info.name) for info in repository.store_list("archives")] fragments = raw_store_objects(repository, "index") caches = raw_store_objects(repository, "cache") assert ids and fragments references = [data for name, data in caches.items() if name.startswith(REFERENCED_BY_ARCHIVE)] checked_packs = caches["checked-packs"] + checked_archives = caches["checked-archives"] + assert len(archive_ids) == 1 assert len(references) == 1 # one archive for name, data in fragments.items(): assert store_hash(data).hexdigest() == name # borg check verifies the fragments by name @@ -47,10 +50,12 @@ def test_index_and_cache_in_the_key_envelope(archivers, request, encryption): assert not any(id in data for data in fragments.values() for id in ids) assert not any(id in references[0] for id in ids) assert not any(pack_id in checked_packs for pack_id in pack_ids) + assert archive_ids[0] not in checked_archives else: assert all(any(id in data for data in fragments.values()) for id in ids) assert any(id in references[0] for id in ids) assert all(pack_id in checked_packs for pack_id in pack_ids) + assert archive_ids[0] in checked_archives def test_references_cache_of_another_archive(archivers, request): diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 0c7edfd062..2321f25509 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -22,7 +22,7 @@ from ..hashindex import ChunkIndex, ChunkIndexEntry from ..platform import get_process_id from ..repository import Repository, MAX_DATA_SIZE, MAX_VALIDATED_META_SIZE, propagate_rsh, rest_serve_command -from ..repository import PackWriter, PackReader, PackTracker, superseded_gap_ranges +from ..repository import ArchiveTracker, PackWriter, PackReader, PackTracker, superseded_gap_ranges from ..repository import SALVAGE_DONE, SALVAGE_INTACT, SALVAGE_NOTHING_AUTHENTICATES from ..repository import SALVAGE_READ_ERROR, SALVAGE_READS_DIFFER, StoreObjectNotFound from ..repoobj import RepoObj, OBJ_MAGIC, OBJ_VERSION, object_validator, whole_object_authenticator @@ -2336,6 +2336,53 @@ def test_check_checked_packs_bound_to_its_name(tmp_path, caplog): assert "Ignoring corrupted checked-packs set." in caplog.text +def test_checked_archives_roundtrip(tmp_path, caplog): + # the archive records are stored apart from the pack records and are bound to their own name. + with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository: + tracker = ArchiveTracker.new(repository) + tracker.record(H(1), ok=True) + tracker.record(H(2), ok=False) + tracker.save() + assert len(PackTracker.load(repository)) == 0 + loaded = ArchiveTracker.load(repository) + assert len(loaded) == 2 + assert loaded.failed_ids() == [H(2)] + assert loaded.is_recent(H(1), 3600) + assert not loaded.is_recent(H(1), 0) + assert not loaded.is_recent(H(2), 3600) # a failed record is never recent + assert not loaded.is_recent(H(3), 3600) + repository.store_store(PackTracker.NAME, repository.store_load(ArchiveTracker.NAME)) + with caplog.at_level(logging.WARNING): + assert len(PackTracker.load(repository)) == 0 + assert "Ignoring corrupted checked-packs set." in caplog.text + loaded.prune({H(2)}) + assert [id for id, _ in ArchiveTracker.load(repository).table.items()] == [H(2)] + loaded.prune(set()) + assert not repository.store.info(ArchiveTracker.NAME).exists + + +@pytest.mark.parametrize("damage", ["none", "corrupt-pack", "missing-pack"]) +def test_check_clears_checked_archives_on_pack_damage(tmp_path, damage): + location = os.fspath(tmp_path / "repo") + with Repository(location, exclusive=True, create=True) as repository: + for x in range(3): + repository.put(H(x), fchunk(b"DATA-%02d" % x, chunk_id=H(x))) + repository.flush() # flush before close persists the index + with Repository(location, exclusive=True) as repository: + tracker = ArchiveTracker.new(repository) + tracker.record(H(9), ok=True) + tracker.save() + pack_key = "packs/" + bin_to_hex(repository.chunks[H(0)].pack_id) + if damage == "corrupt-pack": + pack = bytearray(repository.store_load(pack_key)) + pack[-1] ^= 0xFF + repository.store_store(pack_key, bytes(pack)) + elif damage == "missing-pack": + repository.store_delete(pack_key) + assert repository.check(repair=False) is (damage == "none") + assert len(ArchiveTracker.load(repository)) == (1 if damage == "none" else 0) + + def test_check_partial_rechecks_pack_sorting_before_checked_one(tmp_path): # a partial check verifies a new pack even when its id sorts before an already-checked pack. intact = fchunk(b"INTACT", chunk_id=H(1)) From f89809b49e8e34cfb9352adf40752869c436b067 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Wed, 7 Oct 2026 11:43:03 +0530 Subject: [PATCH 3/5] check tests: assert the changed archive ids only without borg.exe, refs #10025 --- src/borg/testsuite/archiver/check_cmd_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index 2623f7bcc0..c2d6b50147 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -2574,12 +2574,13 @@ def test_check_archives_records_follow_the_archives_directory(archivers, request with open_repository(archiver) as repository: old_ids = set(Manifest.load(repository).archives.ids()) assert {id for id, _ in ArchiveTracker.load(repository).table.items()} == old_ids - # the repair rewrites the item metadata with the default ChunkBuffer size, so the archives get new - # ids, which it records. + # the repair records the archives under the ids they have after it rewrote the item metadata. cmd(archiver, "check", "--archives-only", "--repair", exit_code=0) with open_repository(archiver) as repository: new_ids = set(Manifest.load(repository).archives.ids()) - assert not new_ids & old_ids + if not archiver.EXE: # borg.exe does not see check_cmd_setup's small ChunkBuffer.BUFFER_SIZE + # the rewrite uses the default buffer size, so the item metadata chunks and the ids change. + assert not new_ids & old_ids assert {id for id, _ in ArchiveTracker.load(repository).table.items()} == new_ids assert archive_records(archiver) == {"archive1": 1, "archive2": 1} # compact removes the soft-deleted archives, the next check drops their records. From d6a46e0026a3003a05f08ccfd40d74a83c44ac49 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 12 Oct 2026 19:34:28 +0530 Subject: [PATCH 4/5] check: set the complete flag in the item loop's else clause, refs #10025 --- src/borg/archive.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 69e7553923..a761104e2d 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -3175,15 +3175,16 @@ def check_order(info): raise Exception("Unknown archive metadata version") items_buffer = ChunkBuffer(self.key) items_buffer.write_chunk = add_callback - complete = True + complete = False 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. - complete = False break if "chunks" in item: verify_file_chunks(info.name, item) items_buffer.add(item) + else: + complete = True items_buffer.flush(flush=True) if self.repair: archive.item_ptrs = archive_put_items( From 4c563dc20706aaa0dd130770e6bfd3db314edce1 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 12 Oct 2026 19:34:28 +0530 Subject: [PATCH 5/5] repository: check trackers warn once per run if the store denies the update, refs #10025 --- src/borg/repository.py | 33 +++++++++++-------- .../archiver/restricted_permissions_test.py | 7 ++-- src/borg/testsuite/repository_test.py | 22 +++++++++++++ 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index 484729b12b..c77cfe25fa 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -737,6 +737,9 @@ class CheckTracker: Stored at NAME as the serialized table in the repository key's envelope (see Repository.store_encrypt_store). new() starts an empty tracker, load() reads the stored one. Subclasses set NAME. + + If the store denies writing or deleting NAME, save() and clear() leave the stored object as it is + and log a warning, once per repository object and NAME. """ NAME: str @@ -832,7 +835,10 @@ def save(self): with io.BytesIO() as f: self.table.write(f) data = f.getvalue() - self.repository.store_encrypt_store(self.NAME, data) + try: + self.repository.store_encrypt_store(self.NAME, data) + except StorePermissionDenied: + self._warn_denied() def clear(self): self.table.clear() @@ -840,6 +846,16 @@ def clear(self): self.repository.store_delete(self.NAME) except StoreObjectNotFound: pass + except StorePermissionDenied: + self._warn_denied() + + def _warn_denied(self): + denied = self.repository.check_results_denied + if self.NAME not in denied: + denied.add(self.NAME) + logger.warning( + f"Not updating {self.NAME}: permission denied. The check results stored there stay as they are." + ) class PackTracker(CheckTracker): @@ -871,19 +887,6 @@ class ArchiveTracker(CheckTracker): NAME = "cache/checked-archives" - def save(self): - # a client without write permission for cache/ can still check archives, it just keeps no records. - try: - super().save() - except StorePermissionDenied: - logger.warning(f"Not storing the archive check results: no permission to write {self.NAME}.") - - def clear(self): - try: - super().clear() - except StorePermissionDenied: - logger.warning(f"Not clearing the archive check results: no permission to delete {self.NAME}.") - class _ConfigMissing(Exception): """internal: the store exists, but has no repository config object (see Repository._load_config).""" @@ -1088,6 +1091,8 @@ def __init__( # server-side, so the client does not check them (see above). self.permissions = None if location.proto == "ssh" else permissions self.store_opened = False + # NAMEs of the check results the store denied updating, see CheckTracker. + self.check_results_denied = set() self.version = None self.id = None # the crypto suite of the repository's key, as recorded in the repository config (see save_config): diff --git a/src/borg/testsuite/archiver/restricted_permissions_test.py b/src/borg/testsuite/archiver/restricted_permissions_test.py index f9e77f95d8..892c454b43 100644 --- a/src/borg/testsuite/archiver/restricted_permissions_test.py +++ b/src/borg/testsuite/archiver/restricted_permissions_test.py @@ -125,9 +125,10 @@ def test_repository_permissions_read_only(archivers, request, monkeypatch): extracted_files = os.listdir("output") assert len(extracted_files) > 0 - # Check the archives: this works, but the check results cannot be stored. - output = cmd(archiver, "check", "--archives-only", exit_code=0) - assert "Not storing the archive check results" in output + # Check the repository and the archives: this works, but the check results cannot be stored. + output = cmd(archiver, "check", exit_code=0) + assert output.count("Not updating cache/checked-packs: permission denied.") == 1 + assert output.count("Not updating cache/checked-archives: permission denied.") == 1 # Try to create a new archive, which should fail. with pytest.raises(PermissionDenied): diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 2321f25509..acdfb237ad 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -2361,6 +2361,28 @@ def test_checked_archives_roundtrip(tmp_path, caplog): assert not repository.store.info(ArchiveTracker.NAME).exists +@pytest.mark.parametrize("tracker_cls", [PackTracker, ArchiveTracker]) +def test_check_tracker_store_denied(tmp_path, caplog, tracker_cls): + # a store that denies writing and deleting NAME keeps the stored records, each repository object warns once. + path = str(tmp_path / "repo") + with Repository(path, exclusive=True, create=True) as repository: + tracker = tracker_cls.new(repository) + tracker.record(H(1), ok=True) + tracker.save() + message = f"Not updating {tracker_cls.NAME}: permission denied." + for _ in range(2): + caplog.clear() + with Repository(path, exclusive=True, permissions="read-only") as repository: + with caplog.at_level(logging.WARNING, logger="borg.repository"): + tracker = tracker_cls.load(repository) + tracker.record(H(2), ok=True) + tracker.save() + tracker.prune(set()) # clears + tracker_cls.new(repository).clear() + assert caplog.text.count(message) == 1 + assert [id for id, _ in tracker_cls.load(repository).table.items()] == [H(1)] + + @pytest.mark.parametrize("damage", ["none", "corrupt-pack", "missing-pack"]) def test_check_clears_checked_archives_on_pack_damage(tmp_path, damage): location = os.fspath(tmp_path / "repo")