compact: suggest a full check --repair for unindexed pack bytes, fixes #10429

src/borg/archiver/check_cmd.py

@@ -90,7 +90,7 @@ def do_check(self, args, repository):
             raise CommandError("--repair does not allow --max-duration argument.")
         if args.repair and args.max_age is not None:
             # repair verifies every pack; reusing recorded results during repair needs repository
-            # repair (refs #8572).
+            # repair (refs #10026).
             raise CommandError("--repair does not allow the --max-age option.")
         if args.archives_only and args.max_age is not None:
             # --max-age only affects the repository check; --archives-only skips it.
@@ -301,7 +301,7 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser):
            index from the packs if the index is corrupt, provided every pack matches its
            store hash. If any pack fails its store hash, the repository check leaves the
            index and the packs untouched and reports it; salvaging the intact objects of
-           such a pack is not implemented yet (refs #8572). The rebuild authenticates
+           such a pack is not implemented yet (refs #10026). The rebuild authenticates
            each object's header and metadata with the key, leaves an object that fails
            this out of the index and reports it as an error. Repair mode also removes the
            index entries of the chunks stored in missing packs (packs the index references,

src/borg/archiver/compact_cmd.py

@@ -321,14 +321,17 @@ def compact_packs(self):
                 logger.error(f"{stale_used} of them are still in use: repository data is missing!")
                 set_ec(EXIT_ERROR)
 
-        # bytes no index entry covers. compact_pack reclaims the redundant duplicates among them while
-        # rewriting a pack; reclaiming the rest is tracked in #8572.
+        # unindexed bytes: pack bytes no index entry covers, e.g. the objects of an interrupted borg create.
+        # compact_pack reclaims those that are copies of indexed chunks when it rewrites a pack and keeps the
+        # rest, as "borg check --repair" can recover their objects (#9868). A full check --repair adds these
+        # objects to the index (--repository-only rebuilds it only if it is corrupt), so the next compact
+        # reclaims the unused ones.
         unindexed = sum(total - pack_indexed[pid] for pid, total in pack_total.items() if total > pack_indexed[pid])
         if unindexed:
             logger.info(
-                f"{format_file_size(unindexed)} in pack files is not covered by the index; "
-                "redundant copies are reclaimed on pack rewrite, reclaiming the rest is tracked in "
-                "https://github.com/borgbackup/borg/issues/8572."
+                f"{format_file_size(unindexed)} in pack files is not covered by the index; copies of indexed "
+                'chunks are reclaimed on pack rewrite. Run "borg check --repair" (without --repository-only), '
+                'then "borg compact" to reclaim the rest.'
             )
 
         # packs recorded corrupt in PackTracker that are still in the store
@@ -502,8 +505,8 @@ def build_parser_compact(self, subparsers, common_parser, mid_common_parser):
             ``borg compact`` reclaims objects the chunk index knows about, plus redundant copies of
             indexed chunks (e.g. written by concurrent backups) that it finds while rewriting a pack.
             Other bytes no index entry covers, such as packs left behind by a backup that crashed
-            before recording its objects, are re-indexed by ``borg check --repair`` and reclaimed by
-            the next ``borg compact``.
+            before recording its objects, are re-indexed by ``borg check --repair`` (without
+            ``--repository-only``) and reclaimed by the next ``borg compact``.
 
             ``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 by

src/borg/repository.py

@@ -1489,13 +1489,13 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida
         ArchiveChecker.finish. Packs are verified by the store hash, which is content-addressing rather
         than a MAC, so that check detects accidental corruption but not tampering; the rebuild therefore
         checks every object with validate, see below, refs #9901, #10026. If any pack is corrupt the index
-        is not rebuilt, refs #8572, #10026. Pack ids found corrupt are kept in cache/checked-packs,
+        is not rebuilt, refs #10026. Pack ids found corrupt are kept in cache/checked-packs,
         refs #9696. That object is stored in the key's envelope, too, so check() needs the key (see
         set_key).
 
         A pack recorded corrupt fails the check, also on a partial run that stops before re-reaching
         it. The record clears at the check that finds the pack intact again or gone (removed by
-        compact; TODO: also when repair salvages and drops it, refs #8572); prune() does this from packs/.
+        compact; TODO: also when repair salvages and drops it, refs #10026); prune() does this from packs/.
 
         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
@@ -1790,7 +1790,7 @@ def note_drop():
             if repo_only:
                 logger.error(
                     f"{done} {mode} repository check, corrupt pack(s) found{so_far}; repairing a repository "
-                    "with corrupt packs is not implemented yet (refs #8572)."
+                    "with corrupt packs is not implemented yet (refs #10026)."
                 )
             else:
                 # a full check's archives phase reads archive/item metadata (and file content with

src/borg/testsuite/archiver/compact_cmd_test.py

@@ -373,10 +373,10 @@ def test_compact_keeps_unindexed_waste(tmp_path):
         assert pdchunk(repository.get(H(2))) == b"CCCC"
 
 
-def test_compact_reclaims_indexed_waste_only(tmp_path):
+def test_compact_reclaims_indexed_waste_only(tmp_path, caplog):
     # compact reclaims a pack's indexed-but-unused bytes, but leaves alone a pack whose only waste is
     # unindexed (bytes no index entry covers): those may be live data "borg check --repair" can
-    # recover (#9868).
+    # recover (#9868). It logs how to reclaim them (#10429).
     from ...archiver.compact_cmd import ArchiveGarbageCollector
 
     location = os.fspath(tmp_path / "repo")
@@ -400,8 +400,11 @@ def test_compact_reclaims_indexed_waste_only(tmp_path):
 
         gc = ArchiveGarbageCollector(repository, gc_manifest(repository), stats=False, threshold=10)
         gc.chunks = repository.chunks
-        gc.compact_packs()
+        with caplog.at_level(logging.INFO):
+            gc.compact_packs()
 
+        assert "in pack files is not covered by the index" in caplog.text
+        assert 'Run "borg check --repair" (without --repository-only)' in caplog.text
         pack_names = [info.name for info in repository.store_list("packs")]
         # indexed waste -> compacted, kept object still readable
         assert bin_to_hex(waste_pack) not in pack_names

src/borg/testsuite/repository_test.py

@@ -1874,7 +1874,7 @@ def validate(chunk_id, obj):
 
 def test_check_repair_refuses_when_pack_corrupt(tmp_path):
     # A repair that finds any corrupt pack leaves the index and the pack untouched (no lossy rebuild,
-    # nothing dropped) and fails on a repository-only run, refs #8572, #10026.
+    # nothing dropped) and fails on a repository-only run, refs #10026.
     location = os.fspath(tmp_path / "repo")
     with Repository(location, exclusive=True, create=True) as repository:
         repository.put(H(1), fchunk(b"GOOD-CHUNK", chunk_id=H(1)))
@@ -1995,7 +1995,7 @@ def delete_pack(repository, pack_id):
 def test_check_repair_removes_missing_pack_entries(tmp_path, caplog, repo_only):
     # a repair removes the index entries of the chunks in a missing pack and stores the index. It fails
     # a repository-only run, as the chunks are lost; a full check defers them to the archives phase
-    # (refs #9898, #8572).
+    # (refs #9898).
     location = os.fspath(tmp_path / "repo")
     pack_ids = create_repo_one_pack_per_chunk(location)
     with Repository(location, exclusive=True) as repository: