Re-add old kv679 test and add new test for further dataloss edge case #1296
Conversation
This file contains hidden or 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
This test was part of the original kv679 suite, but at the time of 2.1 it was put in it's own branch. Riak Test is made up of passing tests, and this test (kv679_dataloss_fb.erl) still fails. Adding this back to a branch off develop in preparation for a fix of this issue.
This test shows a dataloss edge case in riak, even in 2.1 with per-key-actor-epochs enabled. The test is a litte convoluted, and is based on a quickcheck counter example, included in the riak_kv/test directory. In short, both this test, and the other kv679_dataloss_fb test, show that even with multiple replicas acking/storing, a single disk error on a single replica is enough to cause acked writes to be silently and permanently lost. For a replicated database, that is bad.
|
I'm going to be opening the PR against riak_kv that provides a fix for both these tests in a short while, and will edit to provide link when I do. In the meantime, feel free to run these against 2.1 and up. |
|
Both tests make sense, and I confirmed they both fail without basho/riak_kv#1643 and pass with it. Still reviewing the riak_kv PR, but the test PR looks good to me. Thanks, Russell! |
|
+1 |
The partition repair test deletes all the data at a partition, and then repairs it from neighbouring partitions. The subset of repaired data that was originally coordinated by the deleted partition's vnode showed up as `notfound` since the latest kv679 changes here basho/riak_kv#1643. The reason is that the fix in the KV repo adds a new actor to the repaired key's vclock. Prior to this change `verify` in partition_repair.erl did a simple equality check on the binary encoded riak_object values. This change takes into account that a new actor may be in the vclock at the repaired vnode, and uses a semantic equality check based on riak_object merge and riak object equal.
|
+1 - merging to fix |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
kv679_dataloss_fb.erltest was part of the original kv679 suite, but at the time of 2.1it was put in it's own branch. See #719 for discussion and decision. In short, Riak Test is made up of passing tests,
and kv679_dataloss_fb.erl still failed. Adding this back to a
branch off develop as I know have a fix of this issue.
There is also a new test,
kv679_dataloss_fb2.erl, that shows another dataloss edge case in riak, even in 2.1 withper-key-actor-epochs enabled. The test is a little convoluted, and is
based on a quickcheck counter example, included in the riak_kv/test
directory. In short, both this test, and the other kv679_dataloss_fb
test, show that even with multiple replicas acking/storing, a single
disk error on a single replica is enough to cause acked writes to be
silently and permanently lost. For a replicated database, that is bad