Validation of the VolatileDB #1568
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kderme
added
WIP
kderme
force-pushed
the
kderme/validate_volatile_db
branch
2 times, most recently
from
4632ec2
to
0ecc3f1
Compare
mrBliss
suggested changes
Feb 6, 2020
Comment on lines
432
to
433
| tryCollectFile hasFS env@VolatileDBEnv{..} slot st@InternalState{..} | ||
| (fileId, fileInfo) = |
There was a problem hiding this comment.
I would revert this change (so it's one line again) and use the following instead:
tryCollectFile ...
....
where
VolatileDBEnv { _dbErr, _tracer } = envThere was a problem hiding this comment.
This is not so easy because VolatileDBEnv is Existential type, but I did the same for InternalState to keep it a single line.
ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl/VolDB.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl/VolDB.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl/VolDB.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/VolatileDB/StateMachine.hs
Outdated
Show resolved
Hide resolved
Comment on lines
641
to
616
| checkBlockIntegrity :: TestBlock -> Bool | ||
| checkBlockIntegrity testBlock = | ||
| hashHeader (testHeader testBlock) == thHash (testHeader testBlock) | ||
| || not (tbIsValid (testBody testBlock)) | ||
|
|
There was a problem hiding this comment.
Isn't this the same as Test.Ouroboros.Storage.TestBlock.testBlockIsValid?
There was a problem hiding this comment.
Almost yes, this also checkes tbIsValid
kderme
force-pushed
the
kderme/validate_volatile_db
branch
2 times, most recently
from
8784596
to
f04ff48
Compare
mrBliss
reviewed
Feb 7, 2020
ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl/VolDB.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/VolatileDB/StateMachine.hs
Outdated
Show resolved
Hide resolved
kderme
force-pushed
the
kderme/validate_volatile_db
branch
6 times, most recently
from
1abf132
to
4b1e9eb
Compare
mrBliss
force-pushed
the
kderme/validate_volatile_db
branch
from
4b1e9eb
to
dda86df
Compare
mrBliss
reviewed
Feb 13, 2020
mrBliss
force-pushed
the
kderme/validate_volatile_db
branch
from
dda86df
to
ba2a97a
Compare
mrBliss
approved these changes
Feb 13, 2020
|
This ready for merging. Thanks @kderme for the validation code. I cleaned up some bits and implemented part 1 of #1304 (comment). |
mrBliss
force-pushed
the
kderme/validate_volatile_db
branch
from
ba2a97a
to
6b60de3
Compare
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Feb 13, 2020
1568: Validation of the VolatileDB r=mrBliss a=kderme Addresses part 2 of #1304 (comment) and more: - there is an option to check if each parsed block is valid, while parsing. - the db now recovers when there is a duplicated slot and ignores invalid filenames. - added tests for this. - added traces. `readIncrementalOffsets` is no longer used anywhere. Worth keeping? 1640: demo chain sync -- cli args r=coot a=coot Co-authored-by: kderme <k.dermenz@gmail.com> Co-authored-by: Thomas Winant <thomas@well-typed.com> Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
|
This PR was included in a batch that timed out, it will be automatically retried |
iohk-bors bot
added a commit
that referenced
this pull request
Feb 13, 2020
1568: Validation of the VolatileDB r=mrBliss a=kderme Addresses part 2 of #1304 (comment) and more: - there is an option to check if each parsed block is valid, while parsing. - the db now recovers when there is a duplicated slot and ignores invalid filenames. - added tests for this. - added traces. `readIncrementalOffsets` is no longer used anywhere. Worth keeping? Co-authored-by: kderme <k.dermenz@gmail.com> Co-authored-by: Thomas Winant <thomas@well-typed.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses part 2 of #1304 (comment) and more:
readIncrementalOffsetsis no longer used anywhere. Worth keeping?