Bugfix/key transform crash #186
Merged
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
Change the default key transformation to only allow binaries. This will
make it have the same assumption that previous user defined key transforms
have had since they were created. It will fail if it gets passed a
{tombstone, Key} tuple, which is the problem being fixed here.
Run a merge with completely expired files, one of which contains a
tombstone. This will trigger the path that passes the {tombstone, Key}
instead of just the Key to the key transformer. This will fail with a
function_clause error as expected.
When merging completely expired files, we fold over them issuing
conditional keydir deletes on the entries of the file. That way, if any
of them are still present in the keydir, they will be deleted now
instead of the usual lazy delete on read, when we detect they are
expired.
Previously, the code was not accounting for the new {tombstone, Key}
return format when folding over the hintfile. A recent change in 1.7.0
added a tombstone bit flag to hintfiles, so now we can tell we are
dealing with a tombstone. The tuple was being passed whole to the user
defined key transformation function, causing a crash. With this change,
we simply skip over tombstones. It makes no sense to try to delete them
from the keydir.
|
+1 5fbb8f7 Reviewed code, accounts for the tombstone response returned by bitcask_fileops:fold_keys. |
borshop
added a commit
that referenced
this pull request
Sep 6, 2014
Bugfix/key transform crash Reviewed-by: jonmeredith
|
@borshop merge |
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.
When merging completely expired files, we fold over them issuing
conditional keydir deletes on the entries of the file. That way, if any
of them are still present in the keydir, they will be deleted now
instead of the usual lazy delete on read, when we detect they are
expired.
Previously, the code was not accounting for the new {tombstone, Key}
return format when folding over the hintfile. A recent change in 1.7.0
added a tombstone bit flag to hintfiles, so now we can tell we are
dealing with a tombstone without inspecting the data file. The tuple was
being passed whole to the userdefined key transformation function, causing
a crash. With this change, we simply skip over tombstones. It makes no
sense to try to delete them from the keydir.
This fixes #185 in the 1.7 branch (Bitcask shipped with Riak 2.0.0).
When merged, this should be ported or merged to the develop branch also.