Skip to content

check: summarize findings and repairs #9891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
charmikadi wants to merge 3 commits into
base: master
Choose a base branch
from
+43 −12
Open

check: summarize findings and repairs #9891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56b79d2
check: summarize archive problems found and repairs Track problem/re…
charmikadi Jul 9, 2026
e4844c3
check: assert summary counts in repair tests
charmikadi Jul 9, 2026
4b7e2a6
check: black-format archive check summary log line
charmikadi Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 37 additions & 12 deletions src/borg/archive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1873,11 +1873,19 @@ class ArchiveChecker:

def __init__(self):
self.error_found = False
self.problems_found = 0
self.repairs_done = 0
self.key = None
# True once repair drops a defect chunk or writes a new one, i.e. once the chunks index no
# longer matches the packs.
self.chunks_modified = False

def _note_problem(self, *, repaired=False):

@ThomasWaldmann ThomasWaldmann Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guess this is important enough to not use a default for repaired, but to require the caller to be always explicit about whether something was repaired or not.

self.error_found = True
self.problems_found += 1
if repaired and self.repair:
self.repairs_done += 1

def check(
self,
repository,
Expand Down Expand Up @@ -1938,16 +1946,18 @@ def check(
repository.get_manifest()
except NoManifestError:
logger.error("Repository manifest is missing.")
self.error_found = True
rebuild_manifest = True
else:
try:
self.manifest = Manifest.load(repository, (Manifest.Operation.CHECK,), key=self.key)
except IntegrityErrorBase as exc:
logger.error("Repository manifest is corrupted: %s", exc)
self.error_found = True
rebuild_manifest = True
if rebuild_manifest:
if self.repair:
self._note_problem(repaired=True)
else:
self._note_problem()
Comment on lines +1957 to +1960

@ThomasWaldmann ThomasWaldmann Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be just 1 line, using repaired=self.repair.

but there is a bigger problem here: it already assumes that the manifest could be rebuilt (repaired), before even calling rebuild_manifest.

self.manifest = self.rebuild_manifest()
# 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:
Expand All @@ -1972,7 +1982,14 @@ def check(
logger.info("Archive consistency check interrupted, no problems found so far.")
raise Error("Got Ctrl-C / SIGINT.")
if self.error_found:
logger.error("Archive consistency check complete, problems found.")
if self.repair and self.repairs_done:
logger.error(
"Archive consistency check complete, %d problem(s) found, %d repaired.",
self.problems_found,
self.repairs_done,
)
Comment on lines +1986 to +1990

@ThomasWaldmann ThomasWaldmann Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't work for the case that problems were found, but 0 repairs were done.

else:
logger.error("Archive consistency check complete, %d problem(s) found.", self.problems_found)
else:
logger.info("Archive consistency check complete, no problems found.")
return self.repair or not self.error_found
Expand Down Expand Up @@ -2031,11 +2048,14 @@ def verify_data(self):
try:
encrypted_data = self.repository.get(chunk_id)
except (Repository.ObjectNotFound, IntegrityErrorBase) as err:
self.error_found = True
errors += 1
logger.error("chunk %s: %s", bin_to_hex(chunk_id), err)
if isinstance(err, IntegrityErrorBase):
defect_chunks.append(chunk_id)
if not self.repair:
self._note_problem()
else:
self._note_problem()
else:
try:
# we must decompress, so it'll call assert_id() in there.
Expand All @@ -2045,10 +2065,11 @@ def verify_data(self):
chunk_id, encrypted_data, decompress=True, ro_type=ROBJ_DONTCARE, assert_id_place="verify_data"
)
except IntegrityErrorBase as integrity_error:
self.error_found = True
errors += 1
logger.error("chunk %s, integrity error: %s", bin_to_hex(chunk_id), integrity_error)
defect_chunks.append(chunk_id)
if not self.repair:
self._note_problem()
pi.finish()
if defect_chunks:
if self.repair:
Expand Down Expand Up @@ -2077,6 +2098,7 @@ def verify_data(self):
self.chunks_modified = True
# drop it from our own index too, so rebuild_archives reports the file it belongs to.
del self.chunks[defect_chunk]
self._note_problem(repaired=True)

@ThomasWaldmann ThomasWaldmann Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, guess we need to think more about what "repaired" means. it just deleted a defective chunk. if that chunk is referenced, this means data loss. so, we do not have the defect chunk anymore, but we also lost the data it originally contained.

else:
logger.warning("chunk %s not deleted, did not consistently fail.", bin_to_hex(defect_chunk))
else:
Expand Down Expand Up @@ -2141,7 +2163,7 @@ def valid_archive(obj):
meta = self.repo_objs.parse_meta(chunk_id, cdata, ro_type=ROBJ_DONTCARE)
except IntegrityErrorBase as exc:
logger.error("Skipping corrupted chunk: %s", exc)
self.error_found = True
self._note_problem()
continue
if meta["type"] != ROBJ_ARCHIVE_META:
continue
Expand All @@ -2151,7 +2173,7 @@ def valid_archive(obj):
meta, data = self.repo_objs.parse(chunk_id, cdata, ro_type=ROBJ_DONTCARE)
except IntegrityErrorBase as exc:
logger.error("Skipping corrupted chunk: %s", exc)
self.error_found = True
self._note_problem()
continue
if meta["type"] != ROBJ_ARCHIVE_META:
continue # should never happen
Expand All @@ -2172,12 +2194,13 @@ def valid_archive(obj):
f"We already have a soft-deleted archives directory entry for {name} {archive_id_hex}."
)
else:
self.error_found = True
if self.repair:
logger.warning(f"Creating archives directory entry for {name} {archive_id_hex}.")
self.manifest.archives.create(name, archive_id, archive.time)
self._note_problem(repaired=True)
else:
logger.warning(f"Would create archives directory entry for {name} {archive_id_hex}.")
self._note_problem()

pi.finish()
if sig_int:
Expand Down Expand Up @@ -2245,7 +2268,7 @@ def verify_file_chunks(archive_name, item):
)
)
record_missing_chunk(archive_name, item.path, chunk_id, size)
self.error_found = True
self._note_problem()
offset += size
if "size" in item:
item_size = item.size
Expand Down Expand Up @@ -2297,7 +2320,7 @@ def missing_chunk_detector(chunk_id):
def report(msg, chunk_id, chunk_no):
cid = bin_to_hex(chunk_id)
msg += " [chunk: %06d_%s]" % (chunk_no, cid) # see "debug dump-archive-items"
self.error_found = True
self._note_problem()
logger.error(msg)

def list_keys_safe(keys):
Expand Down Expand Up @@ -2396,24 +2419,26 @@ def valid_item(obj):
logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})")
if archive_id not in self.chunks:
logger.error(f"Archive metadata block {archive_id_hex} is missing!")
self.error_found = True
if self.repair:
logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.")
self.manifest.archives.delete_by_id(archive_id)
self._note_problem(repaired=True)
else:
logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.")
self._note_problem()
continue
cdata = self.repository.get(archive_id)
try:
_, data = self.repo_objs.parse(archive_id, cdata, ro_type=ROBJ_ARCHIVE_META)
except IntegrityErrorBase as integrity_error:
logger.error(f"Archive metadata block {archive_id_hex} is corrupted: {integrity_error}")
self.error_found = True
if self.repair:
logger.error(f"Deleting broken archive {info.name} {archive_id_hex}.")
self.manifest.archives.delete_by_id(archive_id)
self._note_problem(repaired=True)
else:
logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.")
self._note_problem()
continue
archive = self.key.unpack_archive(data)
archive = ArchiveItem(internal_dict=archive)
Expand Down
6 changes: 6 additions & 0 deletions src/borg/testsuite/archiver/check_cmd_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ def test_corrupted_manifest(archivers, request):
output = cmd(archiver, "check", "-v", "--repair", exit_code=0)
assert "archive1" in output
assert "archive2" in output
assert "problem(s) found" in output
assert "repaired" in output
cmd(archiver, "check", exit_code=0)


Expand Down Expand Up @@ -778,6 +780,8 @@ def test_verify_data(archivers, request, init_args):
assert "The following chunks are missing in the repository:" in output
assert bin_to_hex(chunk.id) in output
assert src_file in output
assert "problem(s) found" in output
assert "repaired" in output

# run with --verify-data again, it will notice the missing chunk.
output = cmd(archiver, "check", "--archives-only", "--verify-data", exit_code=1)
Expand Down Expand Up @@ -869,6 +873,8 @@ def test_corrupted_file_chunk(archivers, request, init_args):
assert "The following chunks are missing in the repository:" in output
assert bin_to_hex(chunk.id) in output
assert src_file in output
assert "problem(s) found" in output
assert "repaired" in output

# run normal check again
cmd(archiver, "check", "--repository-only", exit_code=0)
Expand Down
Loading