check: keep pack check results, add --max-age to reuse them

docs/internals/data-structures.rst

@@ -36,9 +36,12 @@ config/
 
 cache/
   checked-packs
-    repository check progress (partial checks, full checks' checkpointing),
-    the set of packs checked so far this cycle (pack id -> timestamp, result),
-    as a hashtable with an appended integrity hash
+    repository check results (pack id -> timestamp, result), as a hashtable with an
+    appended integrity hash. Records are kept across checks: ``check --max-age``
+    skips packs whose intact record is younger than the given age, which also lets
+    partial checks (``--max-duration``) continue where a previous one stopped.
+    Records of corrupt packs are kept for repair and always re-verified. Records of
+    packs no longer listed in packs/ are pruned when a check finishes.
 
 There is a list of pointers to archive objects in this directory:
 

src/borg/archiver/check_cmd.py

@@ -4,7 +4,7 @@
 from ..archive import ArchiveChecker
 from ..constants import *  # NOQA
 from ..helpers import set_ec, EXIT_WARNING, CancelledByUser, CommandError, IntegrityError
-from ..helpers import yes, ArchiveFormatter, use_iec_units
+from ..helpers import interval, yes, ArchiveFormatter, use_iec_units
 from ..helpers.argparsing import ArgumentParser
 
 from ..logger import create_logger
@@ -44,6 +44,14 @@ def do_check(self, args, repository):
             raise CommandError("--repository-only contradicts the --find-lost-archives option.")
         if args.repair and args.max_duration:
             raise CommandError("--repair does not allow --max-duration argument.")
+        if args.repair and args.max_age:
+            raise CommandError("--repair does not allow the --max-age option.")
+        if args.archives_only and args.max_age:
+            # --max-age only affects the repository check, which --archives-only skips.
+            raise CommandError("--archives-only does not allow the --max-age option.")
+        if args.max_duration and not args.max_age:
+            # partial checks progress by skipping packs whose record is younger than max_age.
+            raise CommandError("--max-duration requires the --max-age option, e.g. --max-age=4w.")
         if args.max_duration and not args.repo_only:
             # when doing a partial repo check, we can only do a low-level check of the repository files.
             # archives check requires that a full repo check was done before and has built/cached a ChunkIndex.
@@ -64,7 +72,8 @@ def do_check(self, args, repository):
             # the repository check has finished, which can take hours.
             ArchiveFormatter.validate_format(format)
         if not args.archives_only:
-            if not repository.check(repair=args.repair, max_duration=args.max_duration):
+            max_age = int(args.max_age.total_seconds()) if args.max_age else 0
+            if not repository.check(repair=args.repair, max_duration=args.max_duration, max_age=max_age):
                 set_ec(EXIT_WARNING)
         if not args.repo_only and not archive_checker.check(
             repository,
@@ -119,17 +128,26 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser):
         repository checks only, or pass ``--archives-only`` to run the archive checks
         only.
 
+        The ``--max-age`` option makes the check reuse the results of previous
+        repository checks: packs whose intact result is younger than the given
+        interval (e.g. ``--max-age=4w``) are skipped, spreading the verification
+        cost over repeated checks. 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`` or ``--repair``.
+
         The ``--max-duration`` option can be used to split a long-running repository
-        check into multiple partial checks. After the given number of seconds, the check
-        is interrupted. The next partial check will continue where the previous one
-        stopped, until the full repository has been checked. Assuming a complete check
-        would take 7 hours, then running a daily check with ``--max-duration=3600``
-        (1 hour) would result in one full repository check per week. Doing a full
-        repository check aborts any previous partial check; the next partial check will
-        restart from the beginning. With partial repository checks you can run neither
-        archive checks, nor enable repair mode. Consequently, if you want to use
-        ``--max-duration`` you must also pass ``--repository-only``, and must not pass
-        ``--archives-only``, nor ``--repair``.
+        check into multiple partial checks. After the given number of seconds, the
+        check is interrupted. Because a verified pack's result is recorded and
+        reused, ``--max-duration`` requires ``--max-age``: the next partial check
+        skips the recently verified packs and continues with the rest, until every
+        pack has a result younger than ``--max-age``. Assuming a complete check
+        would take 7 hours, then running a daily check with ``--max-duration=3600
+        --max-age=1w`` (1 hour) would result in one full repository verification
+        per week. With partial repository checks you can run neither archive
+        checks, nor enable repair mode. Consequently, if you want to use
+        ``--max-duration`` you must also pass ``--repository-only``, and must not
+        pass ``--archives-only``, nor ``--repair``.
 
         **Warning:** Please note that partial repository checks (i.e., running with
         ``--max-duration``) can only perform non-cryptographic checksum checks on the
@@ -219,6 +237,15 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser):
         subparser.add_argument(
             "--find-lost-archives", dest="find_lost_archives", action="store_true", help="attempt to find lost archives"
         )
+        subparser.add_argument(
+            "--max-age",
+            metavar="INTERVAL",
+            dest="max_age",
+            type=interval,
+            default=None,
+            action=Highlander,
+            help="reuse intact-pack check results younger than INTERVAL (e.g. 4w)",
+        )
         subparser.add_argument(
             "--max-duration",
             metavar="SECONDS",

src/borg/constants.py

@@ -71,6 +71,11 @@
 # MAX_OBJECT_SIZE = MAX_DATA_SIZE + len(PUT header)
 MAX_OBJECT_SIZE = MAX_DATA_SIZE + 41  # see assertion at end of repository module
 
+# Clock skew is the difference between the clocks of the machines writing to a repository (seconds).
+# A check result timestamp up to this far in the future still counts as recent; further ahead than
+# this, the pack is re-verified.
+MAX_CLOCK_SKEW = 7200  # [s]
+
 # How many segment files Borg puts into a single directory by default.
 DEFAULT_SEGMENTS_PER_DIR = 1000
 

src/borg/repository.py

@@ -249,11 +249,13 @@ def iter_headers(self):
 
 
 class PackTracker:
-    """Packs verified in the current check cycle, mapping pack_id -> (timestamp, result).
+    """Pack verification results, mapping pack_id -> (timestamp, result).
 
-    A cycle is one full pass over packs/; --max-duration may spread it over several partial checks.
+    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 with a sha256 over it appended.
-    new() starts a cycle, load() resumes the stored one.
+    new() starts an empty tracker, load() reads the stored one.
     """
 
     NAME = "cache/checked-packs"
@@ -313,6 +315,22 @@ def get(self, pack_id):
     def record(self, pack_id, ok):
         self.table[pack_id] = self.Entry(timestamp=int(time.time()), result=int(ok))
 
+    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 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).
+        """
+        # 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]
+        if len(self.table):
+            self.save()
+        else:
+            self.clear()
+
     def save(self):
         with io.BytesIO() as f:
             self.table.write(f)
@@ -739,7 +757,7 @@ def info(self):
         info = dict(id=self.id, version=self.version)
         return info
 
-    def check(self, repair=False, max_duration=0):
+    def check(self, repair=False, max_duration=0, max_age=0):
         """Check repository consistency.
 
         packs/ and index/ objects are named by the sha256 of their content, so a pack or index file
@@ -751,7 +769,15 @@ def check(self, repair=False, max_duration=0):
         rebuild re-reads every pack anyway - so a read-only check just stops and reports it instead of
         continuing. The index is never rebuilt here in any case: reading every pack to do so would be
         far too slow and expensive for a routine (e.g. cron) check. Salvaging good objects out of
-        corrupt packs and dropping those packs is left to repair, refs #8572.
+        corrupt packs and dropping those packs is left to repair, refs #8572. The ids of the packs
+        found corrupt are kept in cache/checked-packs for repair, refs #9696.
+
+        Any pack on record as corrupt fails the check, including on a partial run that stops before
+        re-reaching it. The record clears when the pack verifies intact again, when compact removes
+        the pack, or when repair salvages and drops it (refs #8572).
+
+        max_age (seconds, 0 = verify every pack): skip packs whose intact record is younger than
+        max_age. Check results are always recorded and kept.
         """
 
         def verify(namespace, name):
@@ -775,19 +801,17 @@ def store_list(namespace):
         assert not (repair and partial)
         mode = "partial" if partial else "full"
         logger.info(f"Starting {mode} repository check")
-        if partial:
-            tracker = PackTracker.load(self.store)
-        else:
-            tracker = PackTracker.new(self.store)
-            tracker.clear()  # a full check verifies every pack, so discard the stored cycle
-        if len(tracker):
-            logger.info(f"Continuing check cycle, {len(tracker)} packs already checked.")
-        else:
+        tracker = PackTracker.load(self.store)
+        if not len(tracker):
             logger.info("Starting from beginning.")
+        elif max_age:
+            logger.info(f"{len(tracker)} pack check results on record, reusing those younger than max_age.")
+        else:
+            logger.info(f"{len(tracker)} pack check results on record, verifying every pack.")
         t_start = time.monotonic()
         t_last_checkpoint = t_start
         index_files = index_errors = 0
-        pack_files = pack_errors = 0
+        pack_files = pack_errors = pack_skipped = 0
         # index and packs get separate progress indicators, each running from 0% to 100%.
         # the index is checked first and in full, on partial checks too: it is small, and index errors
         # stop the pack check below.
@@ -817,8 +841,14 @@ def store_list(namespace):
                 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)
-                if entry is not None and entry.result:  # intact in this cycle; a corrupt one is verified again
-                    continue
+                # skip the pack if its recorded intact result is younger than max_age. a future
+                # timestamp (writer clock ahead of ours) also counts as recent, tolerated up to
+                # MAX_CLOCK_SKEW or max_age ahead, whichever is smaller.
+                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
                 pack_files += 1
                 ok = verify("packs", info.name)
                 if not ok:
@@ -831,30 +861,42 @@ def store_list(namespace):
                     logger.info(f"Checkpointing at pack {info.name}.")
                     tracker.save()
                 if partial and now > t_start + max_duration:
-                    logger.info(f"Finished partial repository check, {len(tracker)} packs checked so far.")
-                    tracker.save()
+                    logger.info(f"Finished partial repository check, {len(tracker)} pack check results on record.")
                     break
             else:
-                # scanned all packs without hitting the time limit: the cycle is done, drop the set.
                 if pack_infos:
                     pack_pi.show(current=len(pack_infos))  # finish at 100%
                 logger.info("Finished checking packs.")
-                tracker.clear()
+            tracker.prune({hex_to_bin(info.name) for info in pack_infos})
             pack_pi.finish()
         else:
             # TODO: --repair will rebuild the index from the packs here instead of stopping (refs #8572).
             logger.error("Repository index is corrupted and must be repaired; skipping the pack check.")
         objs_errors = index_errors + pack_errors
-        logger.info(
-            f"Checked {index_files} index files ({index_errors} errors) and {pack_files} packs ({pack_errors} errors)."
+        summary = (
+            f"Checked {index_files} index files ({index_errors} errors) "
+            f"and {pack_files} packs ({pack_errors} errors)."
         )
-        if objs_errors == 0:
+        if pack_skipped:
+            summary += f" Reused {pack_skipped} recent pack check result(s)."
+        logger.info(summary)
+        # every pack recorded corrupt, not only those verified this run; empty if the index is
+        # corrupt, since then no packs were scanned.
+        corrupt_ids = tracker.corrupt_ids() if index_errors == 0 else []
+        if corrupt_ids:
+            # one id per line (the list can be long).
+            logger.error(f"Found {len(corrupt_ids)} corrupt pack(s):")
+            for pack_id in corrupt_ids:
+                logger.error(f"Corrupt pack: {bin_to_hex(pack_id)}")
+        # fail if this run found errors, or any pack is recorded corrupt.
+        problems = objs_errors != 0 or bool(corrupt_ids)
+        if not problems:
             logger.info(f"Finished {mode} repository check, no problems found.")
         elif repair:
             logger.error(f"Finished {mode} repository check, errors found (repository repair not implemented).")
         else:
             logger.error(f"Finished {mode} repository check, errors found.")
-        return objs_errors == 0 or repair
+        return not problems or repair
 
     def list(self, limit=None, marker=None):
         """

src/borg/testsuite/archiver/check_cmd_test.py

@@ -65,6 +65,31 @@ def test_check_usage(archivers, request):
     assert "archive2" in output
 
 
+def test_check_max_age(archivers, request):
+    archiver = request.getfixturevalue(archivers)
+    check_cmd_setup(archiver)
+
+    # --repair and --archives-only do not allow --max-age, and --max-duration requires --max-age.
+    if archiver.FORK_DEFAULT:
+        cmd(archiver, "check", "--repair", "--max-age=1d", exit_code=CommandError().exit_code)
+        cmd(archiver, "check", "--archives-only", "--max-age=1d", exit_code=CommandError().exit_code)
+        cmd(archiver, "check", "--repository-only", "--max-duration=3600", exit_code=CommandError().exit_code)
+    else:
+        with pytest.raises(CommandError):
+            cmd(archiver, "check", "--repair", "--max-age=1d")
+        with pytest.raises(CommandError):
+            cmd(archiver, "check", "--archives-only", "--max-age=1d")
+        with pytest.raises(CommandError):
+            cmd(archiver, "check", "--repository-only", "--max-duration=3600")
+
+    # a check records its results, a later one with --max-age reuses them.
+    output = cmd(archiver, "check", "-v", "--repository-only", exit_code=0)
+    assert "Starting full repository check" in output
+    output = cmd(archiver, "check", "-v", "--repository-only", "--max-age=4w", exit_code=0)
+    assert "reusing those younger than max_age" in output
+    assert "no problems found" in output
+
+
 def test_date_matching(archivers, request):
     archiver = request.getfixturevalue(archivers)
     check_cmd_setup(archiver)