dolysis/rocksdb
3
0
Fork 0
mirror of https://github.com/facebook/rocksdb.git synced 2024-11-29 18:33:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't run background jobs (flush, compactions) when bg_error_ is set

Browse source 
Summary:
If bg_error_ is set, that means that we mark DB read only. However, current behavior still continues the flushes and compactions, even though bg_error_ is set.

On the other hand, if bg_error_ is set, we will return Status::OK() from CompactRange(), although the compaction didn't actually succeed.

This is clearly not desired behavior. I found this when I was debugging t5132159, although I'm pretty sure these aren't related.

Also, when we're shutting down, it's dangerous to exit RunManualCompaction(), since that will destruct ManualCompaction object. Background compaction job might still hold a reference to manual_compaction_ and this will lead to undefined behavior. I changed the behavior so that we only exit RunManualCompaction when manual compaction job is marked done.

Test Plan: make check

Reviewers: sdong, ljin, yhchiang

Reviewed By: yhchiang

Subscribers: leveldb

Differential Revision: https://reviews.facebook.net/D23223
This commit is contained in:
Igor Canadi 2014-09-11 16:24:16 -07:00
parent a9639bda84
commit 9c0e66ce98
1 changed files with 19 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
db/db_impl.cc
View file

@ -1891,7 +1891,10 @@ Status DBImpl::RunManualCompaction(ColumnFamilyData* cfd, int input_level,
Log(db_options_.info_log, "[%s] Manual compaction starting",
cfd->GetName().c_str());
while (!manual.done && !shutting_down_.Acquire_Load() && bg_error_.ok()) {
// We don't check bg_error_ here, because if we get the error in compaction,
// the compaction will set manual.status to bg_error_ and set manual.done to
// true.
while (!manual.done) {
assert(bg_manual_only_ > 0);
if (manual_compaction_ != nullptr) {
// Running either this or some other manual compaction
@ -2041,6 +2044,11 @@ Status DBImpl::BackgroundFlush(bool* madeProgress,
DeletionState& deletion_state,
LogBuffer* log_buffer) {
mutex_.AssertHeld();
if (!bg_error_.ok()) {
return bg_error_;
}
// call_status is failure if at least one flush was a failure. even if
// flushing one column family reports a failure, we will continue flushing
// other column families. however, call_status will be a failure in that case.
@ -2228,6 +2236,16 @@ Status DBImpl::BackgroundCompaction(bool* madeProgress,
bool is_manual = (manual_compaction_ != nullptr) &&
(manual_compaction_->in_progress == false);
if (!bg_error_.ok()) {
if (is_manual) {
manual_compaction_->status = bg_error_;
manual_compaction_->done = true;
manual_compaction_->in_progress = false;
manual_compaction_ = nullptr;
}
return bg_error_;
}
if (is_manual) {
// another thread cannot pick up the same work
manual_compaction_->in_progress = true;