add code to clean up empty data and hintfiles #137
Conversation
|
A unit test has been requested, I'll work on that tomorrow. |
If I have an EUnit test that fails like this, for example:
*failed*
in function bitcask:do_put/5 (src/bitcask.erl, line 1242)
in call from bitcask:put/3 (src/bitcask.erl, line 244)
in call from bitcask:'-gh137_regression_test/0-lc$^0/1-0-'/2 (src/bitcask.erl, line 2024)
in call from bitcask:gh137_regression_test/0 (src/bitcask.erl, line 2024)
**error:{badmatch,{error,{badmatch,{error,eexist}}}}
... also yields this error complaint that really isn't an error:
=ERROR REPORT==== 5-Feb-2014::13:42:11 ===
** Generic server <0.258.0> terminating
** Last message in was {'DOWN',#Ref<0.0.0.6952>,process,<0.230.0>,normal}
** When Server state == {state,undefined,undefined}
** Reason for termination ==
** {{badmatch,{error,badarg}},
[{bitcask_file,handle_info,2,[{file,"src/bitcask_file.erl"},{line,170}]},
{gen_server,handle_msg,5,[{file,"gen_server.erl"},{line,604}]},
{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,239}]}]}
Don't use an "ok = file:close...." pattern match here.
|
Evan, I've added:
|
|
This looks good 👍 |
|
I share your worry about removal on opening. Would it make more sense to just move the file? Prepend bad- so that the file id info is retained? |
|
I've a patch that would delete files only if called by init_keydir(), when we know that we're not merging. But that patch is upset/full-of-merge-conflicts when the still-pending tombstone management branch review is finally finished. ... There are so many 1.6 branch commits that need to be reconciled with the ts mgmt branch, my head is about to explode. ... Awww heck, I'll add the patch here. If you like it, we'll keep it, otherwise I'll either apply an un-patch or reset the branch back to last +1'ed commit, 73242fc. |
|
I think that's reasonable. |
add code to clean up empty data and hintfiles
This fix is meant to fix #136. I am concerned about the potential for this to delete something in error, but I am not sure how best to test it/trigger it. I can't think of anything that would cause this to delete accessible data, but that doesn't mean there isn't a way.
There is another potential fix that doesn't involve deletion, but that will only work in 2.0 after #128 lands.
@slfritchie @engelsanchez if you could take a look, this is considered a blocker for 1.4.8.