basho / bitcask

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

Updates create unnecessary tombstones #196

Closed engelsanchez closed 10 years ago

engelsanchez commented 10 years ago

With the new way of handling tombstones starting in Bitcask 1.7.0, puts may first write a tombstone for a value they are updating if the previous version of that value is not in the current write file. The idea is to pair up each tombstone with a file, so that merges know exactly when that tombstone can be dropped. Each tombstone will disappear only when the file it points to, which contains all the values it is obsoleting, disappears.

Unfortunately this commit changed the update tombstone logic. Now tombstones are written on updates to a value that resides in the current write file. Updating a value N times (assuming no file wrapping), creates N superfluous tombstones.

You can confirm this by running the following commands, making sure eper is in the code path:

1> B = bitcask:open("test1", [read_write]).
#Ref<0.0.0.50>
2> bitcask:put(B, <<"k">>, <<"v">>).
3> redbug:start("bitcask_fileops:write/4", [{msgs, 1000},{time,300000}]).
{37,1}
4> bitcask:put(B, <<"k">>, <<"v">>).

14:40:44 <0.32.0>({erlang,apply,2}) {bitcask_fileops,write,
                                     [{filestate,read_write,
                                       "test1/2.bitcask.data",2,<0.54.0>,
                                       <0.55.0>,1175659381,53,37,19,128246162},
                                      <<"k">>,
                                      <<98,105,116,99,97,115,107,95,116,111,
                                        109,98,115,116,111,110,101,50,0,0,0,2>>,
                                      1410806444]}

14:40:44 <0.32.0>({erlang,apply,2}) {bitcask_fileops,write,
                                     [{filestate,read_write,
                                       "test1/2.bitcask.data",2,<0.54.0>,
                                       <0.55.0>,1096186912,90,53,19,
                                       1175659381},
                                      <<"k">>,<<"v">>,1410806444]}
ok

Where the binary has the <<"bitcask_tombstone2">> prefix.

slfritchie commented 10 years ago

197 has been merged