basho/bitcask

Race listing bitcask files in directory

Closed this issue · 2 comments

I believe there is a race in bitcask:readable_and_setuid_files/1 and hence bitcask:readable_files/1 that may allow the currently active write file and/or the currently active merge output files to be returned in the list.

The function tries to remove those from the directory list, but it does it in two stages. First, it reads the current merge and write files from the lock files in the directory. It then lists all the bitcask data files in the directory and removes the above from the list. But, the writer process may have moved on to another write file after filling up the previous one. A merge process could have advanced to another merge output file. Any of those new fies could be listed and not filtered out, which could lead to Bitcask trying to merge the current write, for example.

A solution would be to check again after the list has been generated. If the write or merge files have changed, try again until we can perform both steps without any changes in between that could lead to trouble. Notice that this is what this crazy code in bitcask.erl#L1333-L1341 might have been intending to do. But alas, it's just crazy.

Notice also that merges in Riak do not use this code path. bitcask:needs_merge/2 builds the file list using bitcask:summary_info/1. That file list is built from file stats in the keydir, from which the current file is removed. At worst, it will not include a just finished data file, but will not include the new write file by mistake.

PR #198 contains a failing test demonstrating this problem and a fix.

Fixed in #198 and merged to the 1.7 branch.