andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

fix bug in merge_iterator when data race happens

Browse Source 
Summary:
core dump when run
`./db_stress --max_background_compactions=1 --max_write_buffer_number=3 --sync=0 --reopen=20 --write_buffer_size=33554432 --delpercent=5 --log2_keys_per_lock=10 --block_size=16384 --allow_concurrent_memtable_write=1 --test_batches_snapshots=0 --max_bytes_for_level_base=67108864 --progress_reports=0 --mmap_read=1 --kill_prefix_blacklist=WritableFileWriter::Append,WritableFileWriter::WriteBuffered --writepercent=35 --disable_data_sync=0 --readpercent=50 --subcompactions=3 --ops_per_thread=20000000 --memtablerep=skip_list --prefix_size=0 --target_file_size_multiplier=1 --column_families=1 --db=/dev/shm/rocksdb/rocksdb_crashtest_whitebox --threads=32 --disable_wal=0 --open_files=500000 --destroy_db_initially=0 --target_file_size_base=16777216 --nooverwritepercent=1 --iterpercent=10 --max_key=100000000 --prefixpercent=0 --use_clock_cache=false --kill_random_test=189 --cache_size=1048576 --verify_checksum=1`
Actually the relevant flag is `--threads`, data race when --thread > 1 cause problem.
It is possible that multiple
threads read/write memtable simultaneously. After one thread
calls Prev(), another thread may insert a new key just between
the current key and the key next, which may cause the
assert(current_ == CurrentForward()) failure when the first
thread calls Next() again if in prefix seek mode

Test Plan: rerun db_stress with >1 thread / make all check -j64

Reviewers: sdong, andrewkr, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62979
This commit is contained in:
Aaron Gao 2016-08-30 22:19:42 -07:00
parent b18f9c9eac
commit db74b1a219
1 changed files with 15 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
table/merger.cc
View File

@ -141,6 +141,9 @@ class MergingIterator : public InternalIterator {
if (&child != current_) { if (&child != current_) {
if (prefix_extractor_ == nullptr) { if (prefix_extractor_ == nullptr) {
child.Seek(key()); child.Seek(key());
if (child.Valid() && comparator_->Equal(key(), child.key())) {
child.Next();
}
} else { } else {
// only for prefix_seek_mode // only for prefix_seek_mode
// we should not call Seek() here // we should not call Seek() here
@ -150,7 +153,14 @@ class MergingIterator : public InternalIterator {
child.SeekToFirst(); child.SeekToFirst();
} }
} }
if (child.Valid() && comparator_->Equal(key(), child.key())) { // This condition is needed because it is possible that multiple
// threads read/write memtable simultaneously. After one thread
// calls Prev(), another thread may insert a new key just between
// the current key and the key next, which may cause the
// assert(current_ == CurrentForward()) failure when the first
// thread calls Next() again if in prefix seek mode
while (child.Valid() &&
comparator_->Compare(key(), child.key()) >= 0) {
child.Next(); child.Next();
} }
} }
@ -217,6 +227,10 @@ class MergingIterator : public InternalIterator {
TEST_SYNC_POINT("MergeIterator::Prev:BeforeSeekToLast"); TEST_SYNC_POINT("MergeIterator::Prev:BeforeSeekToLast");
child.SeekToLast(); child.SeekToLast();
} }
while (child.Valid() &&
comparator_->Compare(key(), child.key()) <= 0) {
child.Prev();
}
} }
if (child.Valid()) { if (child.Valid()) {