check: summarize findings and repairs - #9891
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9891 +/- ##
==========================================
- Coverage 86.95% 84.77% -2.18%
==========================================
Files 101 101
Lines 17983 18013 +30
Branches 2737 2743 +6
==========================================
- Hits 15637 15271 -366
- Misses 1639 2045 +406
+ Partials 707 697 -10 ☔ View full report in Codecov by Harness. |
|
@charmikadi Thanks for helping with this! Just be aware that @mr-raj12 is currently working on packs and that might need bigger changes in |
|
@mr-raj12 Check this, please. |
|
Thanks for working on this, @charmikadi! My repo repair PR #10048 is landing soon and it rewrites this same check() finish region in repository.py (the "repository repair not implemented" line goes away) and also ArchiveChecker.finish(), so this PR currently conflicts with master there and it's around 330 commits behind. Would suggest rebasing onto latest master and narrowing this to only the archive-side summary for now, and we can add the repository-side repairs count once #10048 is merged. I'll reuse your counting approach and credit you when I do the follow-up. |
…air counts in ArchiveChecker and print them at the end of the archive consistency check.
14337fe to
4b7e2a6
Compare
ThomasWaldmann
left a comment
ThomasWaldmann
left a comment
There was a problem hiding this comment.
thanks for working on this, some feedback.
please note that @mr-raj12 should be still working on borg check --repair, so this is a potentially ongoing merge conflict.
| if self.repair: | ||
| self._note_problem(repaired=True) | ||
| else: | ||
| self._note_problem() |
There was a problem hiding this comment.
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.
| logger.error( | ||
| "Archive consistency check complete, %d problem(s) found, %d repaired.", | ||
| self.problems_found, | ||
| self.repairs_done, | ||
| ) |
There was a problem hiding this comment.
this doesn't work for the case that problems were found, but 0 repairs were done.
| 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) |
There was a problem hiding this comment.
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.
| # longer matches the packs. | ||
| self.chunks_modified = False | ||
|
|
||
| def _note_problem(self, *, repaired=False): |
There was a problem hiding this comment.
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.
3 participants
Description
borg checkalready reports individual problems as it finds them, but it was hard to tell at a glance how many issues there were overall, or how many--repairactually fixed.This adds a short summary at the end of the check:
Also updated a few repair tests to cover the new summary lines.
Fixes #2397
Checklist
master(or maintenance branch if only applicable there)toxor the relevant test subset)