rocksdb Prevent a case of WriteBufferManager flush thrashing

Previously, the flushes triggered by WriteBufferManager could affect the same CF repeatedly if it happens to get consecutive writes. Such flushes are not particularly useful for reducing memory usage since they switch nearly-empty memtables to immutable while they've just begun filling their first arena block. In fact they may not even reduce the mutable memory count if they involve replacing one mutable memtable containing one arena block with a new mutable memtable containing one arena block. Further, if such switches happen even a few times before a flush finishes, the immutable memtable limit will be reached and writes will stall.

This PR adds a heuristic to not switch memtables to immutable for CFs that already have one or more immutable memtables awaiting flush. There is a memory usage regression if the user continues writing to the same CF, that DB does not have any CFs eligible for switching, flushes are not finishing, and the WriteBufferManager was constructed with allow_stall=false. Before, it would grow by switching nearly empty memtables until writes stall. Now, it would grow by filling memtables until writes stall. This feels like an acceptable behavior change because users who prefer to stall over violate the memory limit should be using allow_stall=true, which is unaffected by this PR.

Test Plan:

Command:

rm -rf /dev/shm/dbbench/ && TEST_TMPDIR=/dev/shm ./db_bench -benchmarks=fillrandom -num_multi_db=8 -num_column_families=2 -write_buffer_size=4194304 -db_write_buffer_size=16777216 -compression_type=none -statistics=true -target_file_size_base=4194304 -max_bytes_for_level_base=16777216

rocksdb.db.write.stall count before this PR: 175
rocksdb.db.write.stall count after this PR: 0

Feb 03 '20 20:02 ajkr

@cheng-chang I talked to @siying and he prefers WriteBufferManager to be strict, while this PR would make it less strict. so I am not sure this change is desirable, although we didn't discuss this PR specifically.

Feb 28 '20 21:02 ajkr

@ajkr I thought it should be imported, in that case, I'll discard the diff in Phabricator.