Bitcask keydir remembers deleted files about bitcask

[slfritchie]

One of the causes of bursts of activity is the Riak KV backend API status() call, which in turn calls bitcask:status(). summary_info() is filtering out files from the keydir that no longer exist, but it has no mechanism in this function to feed back missing files to the keydir.

bitcask/src/bitcask.erl

Line 830 in 3fac8e9

Summary = lists:keysort(1, [S || S <- Summary0,

Note that needs_merge(), which is also a known latency outlier cause, is also calling summary_info().

bitcask/src/bitcask.erl

Line 653 in 3fac8e9

needs_merge(Ref) ->

[slfritchie]

Hrm, this appears to be long-time bitcask behavior, that there is no way for the merge process to tell the keydir that a file has been deleted, so its fstats entry is kept forever? And that

bitcask/src/bitcask.erl

Line 830 in 3fac8e9

Summary = lists:keysort(1, [S || S <- Summary0,

is The Way(tm) to ignore those entries? Oi oi. I'm thinking it's much much better to tell the keydir that a file is gone.

With the patch below:

diff --git a/src/bitcask.erl b/src/bitcask.erl
index aa16822..216e226 100644
--- a/src/bitcask.erl
+++ b/src/bitcask.erl
@@ -787,6 +787,16 @@ summary_info(Ref) ->
     %% and is only an estimate/snapshot.
     {KeyCount, _KeyBytes, Fstats, _IterStatus} =
         bitcask_nifs:keydir_info(State#bc_state.keydir),
+try
+  %%{QQQa, QQQb, QQQc} = now(),
+  %%QQQpath = "/tmp/" ++ filename:basename(State#bc_state.dirname) ++ "." ++ lists:flatten(io_lib:format("~p.~p.~p", [QQQa, QQQb, QQQc])),
+  QQQpath = "/tmp/" ++ filename:basename(State#bc_state.dirname),
+  {ok, QQQfh} = file:open(QQQpath, [write]),
+  [io:format(QQQfh, "~p\n", [QQQxxx]) || QQQxxx <- [KeyCount, _KeyBytes, Fstats, _IterStatus]],
+  file:close(QQQfh)
+catch __X:__Y ->
+error_logger:warning_msg("DEBUG exception at ~p ~p: ~p ~p\n", [?MODULE, ?LINE, __X, __Y])
+end,

     %% We want to ignore the file currently being written when
     %% considering status!

... I see this for one vnode (but all show the same pattern) in a Riak workload
that is constantly clobbering a small number of keys:

440
9240
[{84,105,127,159471,192877,1382691404,1382691406},
 {83,18,24,27336,36448,1382691341,1382691380},
 {82,242,567,367518,861120,1382691393,1382691404},
 {81,75,690,113909,1047940,1382691380,1382691393},
 {80,0,690,0,1047926,1382691368,1382691380},
 {79,0,690,0,1047950,1382691356,1382691368},
 {78,0,690,0,1047930,1382691344,1382691356},
 {77,0,690,0,1047934,1382691332,1382691344},
 ... yup, casks 76 down to 4 are also 0 live keys & live bytes ...
 {3,0,690,0,1047928,1382690525,1382690533},
 {2,0,690,0,1047948,1382690517,1382690525},
 {1,0,690,0,1047943,1382690510,1382690517}]
{<<0,0,0,0,0,0,0,0>>,0,false}

• If a file is removed, and the keydirs->fstats says live_keys and live_bytes is 0, then we ought definitely get rid of it.
• If a file is removed, and the keydirs->fstats says either of live_keys or live_bytes is not 0, then we have a bug?

What if we deleted the keydirs->fstats entry as soon as live_keys and live_bytes dropped to 0? My sinus-infection-wracked brain isn't clear on whether that's a good idea or not.

[evanmcc]

I am tempted to add a new nif fun called keydir_trim_fstats([integer()]), then since erlang-side bitcask is (should be?) the authority on file existence, have it trim the those files_ids when it updates the list of open files post-merge, around here:
https://github.com/basho/bitcask/blob/develop/src/bitcask.erl#L667

Since we don't log from the keydir code, this would make it easier to do something like:

    case bitcask_nifs:keydir_trim_fstats(DeletedFileList) of
        ok -> ok;
        {error, FileList} ->
            lager:error("bitcask keydir reports deleted files ~p had live keys or data")
    end,

Which would be hard or API-deforming if we tried to do it from the delete.

[evanmcc]

Another reason to not do automatic removal: in my first commit on 1.6.3-release...pevm-status-workaround (my attempt at a quick beam-only hotpatch), I found that since the summary is the only source for fstats, if 0-key or 0-data stuff is gone before it's merged, it'll never get merged.