feat(checker): Firewood checker framework #936
Conversation
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ron Kuris <[email protected]>
|
This diff is NOT ready for merge - just here to test with conventional commit requirements |
rkuris
left a comment
rkuris
left a comment
There was a problem hiding this comment.
I didn't review the range map thoroughly yet but there are a few things to work on in this review. It's very close!
The errors need to contain more information, enough for us to figure out exactly what is wrong with the database. See if you can make these errors a bit cleaner by including additional information in them.
|
Depends on: #957 |
| /// Returns a [`CheckerError`] if the database is inconsistent. | ||
| // TODO: report all errors, not just the first one | ||
| // TODO: add merkle hash checks as well | ||
| pub fn check(&self) -> Result<(), CheckerError> { |
There was a problem hiding this comment.
High level: this only returns a single error and only the first error we find. Ideally, we should be returning every error we can discover and only abort the entire scan when we find an error we cannot skip past. And even then, we should include all the errors we discovered up until that point. If there is something we can fix, then we will want to do that in bulk instead of (1) running fsck, (2) getting and error, (3) fixing it, and (1) running fsck again until (4) we get an error we can't fix or it's clean.
This could potentially be done using a callback instead of collecting everything into a vec. Have the caller of check pass in something that implements a trait that receives each error we discover as we discover it.
There was a problem hiding this comment.
We are aware of this issue but at the same time some errors may be detrimental and it might not make sense to continue running the checker.
My plan for this PR (and the next one) is to get a simplistic checker that catches only the first error it encounters so that we know all the errors that can happen. In future PRs we can decide which errors can be fixed and which ones are detrimental.
No description provided.