Chain DB: fix iterator bug and add tests #810
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
mrBliss
commented
Jul 22, 2019
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/ChainDB/Iterator.hs
Show resolved
Hide resolved
nfrisby
reviewed
Jul 22, 2019
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/ImmutableDB/Mock.hs
Show resolved
Hide resolved
nfrisby
reviewed
Jul 22, 2019
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/ChainDB/Iterator.hs
Outdated
Show resolved
Hide resolved
nfrisby
reviewed
Jul 22, 2019
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/ChainDB/Iterator.hs
Show resolved
Hide resolved
mrBliss
force-pushed
the
mrBliss/chaindb-iterator-tests
branch
from
bb0a91a to
240d964
Compare
nfrisby
reviewed
Jul 22, 2019
mrBliss
force-pushed
the
mrBliss/chaindb-iterator-tests
branch
from
240d964 to
d0e55f9
Compare
Instead of relying on the whole ChainDB, rely only on the parts actually needed. This way, simple mocks of the ImmutableDB and the VolatileDB can be used to easily test the interesting edge cases.
This mock ImmutableDB will be used for the ChainDB iterator tests. No mock implementation existed yet (as opposed to the VolatileDB), because the ImmutableDB API does not mention `STM`, which means that we could provide a pure implementation of the ImmutableDB to use as the model in the q-s-m tests.
mrBliss
force-pushed
the
mrBliss/chaindb-iterator-tests
branch
from
2c30f29 to
77cceff
Compare
mrBliss
changed the title
mrBliss
force-pushed
the
mrBliss/chaindb-iterator-tests
branch
from
77cceff to
9c11d7f
Compare
nfrisby
reviewed
Jul 23, 2019
ouroboros-consensus/test-storage/Test/Ouroboros/Storage/ChainDB/Iterator.hs
Outdated
Show resolved
Hide resolved
Currently, these are only unit tests. In the future, we will write a generator and turn them into property tests.
When streaming from both the ImmutableDB and the VolatileDB, we need to know the switchover point, i.e. the tip of the ImmutableDB. Previously, we were reading that whole block from disk to figure out its hash, while we already had the necessary information in memory: the anchor point of the in-memory chain fragment refers to the tip of the ImmutableDB (see the invariant of 'cdbChain'). We now take advantage of this information and avoid one extra block read.
This fixes the bug triggered by the `prop_773_bug` test (see its docstring for more information) in `Test.Ouroboros.Storage.ChainDB.Iterator`.
mrBliss
force-pushed
the
mrBliss/chaindb-iterator-tests
branch
from
9c11d7f to
952cfe3
Compare
mrBliss
added a commit
that referenced
this pull request
Jul 26, 2019
PR #797 was recently merged, which no longer compiled after merging with master because of PR #810. CI did not signal this because there were no merge conflicts and it never rebuilt PR #797 against the updated master. To avoid this in the future, use `bors r+`, which first tries to build the merged PR (as opposed to the unmerged PR branch) before actually pushing to master.
Merged
iohk-bors bot
added a commit
that referenced
this pull request
Jul 26, 2019
831: Fix broken master r=nc6 a=mrBliss PR #797 was recently merged, which no longer compiled after merging with master because of PR #810. CI did not signal this because there were no merge conflicts and it never rebuilt PR #797 against the updated master. To avoid this in the future, use `bors r+`, which first tries to build the merged PR (as opposed to the unmerged PR branch) before actually pushing to master. Co-authored-by: Thomas Winant <[email protected]>
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.
No description provided.