basho / bitcask

because you need another a key/value storage engine
1.28k stars 171 forks source link

Operation aborts if any file contains a corrupt key [JIRA: RIAK-2371] #230

Open joecaswell opened 8 years ago

joecaswell commented 8 years ago

The bitcask backend wraps the key transformation function KT() in a try/catch each time it is called:

K = try KT(K0) catch TxErr -> {key_tx_error, TxErr} end,

However, the TxErr will only match explicitly thrown exceptions. In the case of riak_kv_bitcask_backend:key_transform_to_1/1, if an invalid key is encountered the exception thrown is:

{error,function_clause,[{riak_kv_bitcask_backend,key_transform_to_1,[<<>>],[{file,"src/riak_kv_bitcask_backend.erl"},{line,99}]}

which is not caught by the above pattern.

Proper fix is to rework every instances where a key transform is called to use the Class:Exception pattern so exceptions are actually caught.

As an interim workaround, adding a catch-all clause to the transform functions that explicitly throws an exception would allow the existing catch pattern to match for many cases:

key_transform_to_1(<<?VERSION_1:7, _:1, _Rest/binary>> = Key) ->
    Key;
key_transform_to_1(<<131:8,_Rest/bits>> = Key0) ->
    {Bucket, Key} = binary_to_term(Key0),
    make_bk(?VERSION_BYTE, Bucket, Key);
key_transform_to_1(BadKey) ->
    throw({invalid_key, BadKey}).`
evanmcc commented 8 years ago

I wish that was the first time that particular typo had bitten me, but I suppose that I'll never learn.