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

Fix the potential deadlock in WriteImplWALOnly and UnorderedWriteMemtable (#7199)

Browse Source 
Summary:
Pointed out by https://github.com/facebook/rocksdb/issues/7197 , there is a double lock in WriteImplWALOnly.
Also find another deadlock in UnorderedWriteMemtable. Move the check after switch_all_.notify_all().

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7199

Test Plan: pass make check

Reviewed By: anand1976

Differential Revision: D22961714

Pulled By: zhichao-cao

fbshipit-source-id: 0707922dc50d28ea141a15a8cdcbd1c8993ea0d8
This commit is contained in:
Zhichao Cao 2020-08-07 11:27:14 -07:00 committed by Facebook GitHub Bot
parent 56f468b356
commit b79f13b2aa
2 changed files with 17 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
db/db_impl/db_impl.h
View File

@ -1554,6 +1554,10 @@ class DBImpl : public DB {
uint64_t* log_used,
SequenceNumber* last_sequence, size_t seq_inc);
// Used by WriteImpl to update bg_error_ if paranoid check is enabled.
// Caller must hold mutex_.
void WriteStatusCheckOnLocked(const Status& status);
// Used by WriteImpl to update bg_error_ if paranoid check is enabled.
void WriteStatusCheck(const Status& status);

16
db/db_impl/db_impl_write.cc
View File

@ -623,8 +623,6 @@ Status DBImpl::UnorderedWriteMemtable(const WriteOptions& write_options,
0 /*log_number*/, this, true /*concurrent_memtable_writes*/,
seq_per_batch_, sub_batch_cnt, true /*batch_per_txn*/,
write_options.memtable_insert_hint_per_batch);
WriteStatusCheck(w.status);
if (write_options.disableWAL) {
has_unpersisted_data_.store(true, std::memory_order_relaxed);
}
@ -639,6 +637,7 @@ Status DBImpl::UnorderedWriteMemtable(const WriteOptions& write_options,
std::lock_guard<std::mutex> lck(switch_mutex_);
switch_cv_.notify_all();
}
WriteStatusCheck(w.status);
if (!w.FinalStatus().ok()) {
return w.FinalStatus();
@ -689,7 +688,7 @@ Status DBImpl::WriteImplWALOnly(
InstrumentedMutexLock l(&mutex_);
bool need_log_sync = false;
status = PreprocessWrite(write_options, &need_log_sync, &write_context);
WriteStatusCheck(status);
WriteStatusCheckOnLocked(status);
}
if (!status.ok()) {
WriteThread::WriteGroup write_group;
@ -830,6 +829,17 @@ Status DBImpl::WriteImplWALOnly(
return status;
}
void DBImpl::WriteStatusCheckOnLocked(const Status& status) {
// Is setting bg_error_ enough here? This will at least stop
// compaction and fail any further writes.
// Caller must hold mutex_.
mutex_.AssertHeld();
if (immutable_db_options_.paranoid_checks && !status.ok() &&
!status.IsBusy() && !status.IsIncomplete()) {
error_handler_.SetBGError(status, BackgroundErrorReason::kWriteCallback);
}
}
void DBImpl::WriteStatusCheck(const Status& status) {
// Is setting bg_error_ enough here? This will at least stop
// compaction and fail any further writes.