Skip to content

Commit

Permalink
FlushMemTable return ok but memtable does not synchronize flush (#8173)
Browse files Browse the repository at this point in the history
Summary:
Fix #8046 : FlushMemTable return ok but memtable does not synchronize flush. The way to fix it is to expose RecoveryError.

Pull Request resolved: #8173

Reviewed By: ajkr

Differential Revision: D31674552

Pulled By: jay-zhuang

fbshipit-source-id: 9d16b69ba12a196bb429332ec8224754de97773d
  • Loading branch information
zhuchong0329 authored and facebook-github-bot committed Jan 12, 2022
1 parent 0376869 commit 5f2b661
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
3 changes: 3 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
### Behavior Changes
* `DB::DestroyColumnFamilyHandle()` will return Status::InvalidArgument() if called with `DB::DefaultColumnFamily()`.

### Bug Fixes
* Fix a bug that FlushMemTable may return ok even flush not succeed.

## 6.28.0 (2021-12-17)
### New Features
* Introduced 'CommitWithTimestamp' as a new tag. Currently, there is no API for user to trigger a write with this tag to the WAL. This is part of the efforts to support write-commited transactions with user-defined timestamps.
Expand Down
14 changes: 10 additions & 4 deletions db/db_impl/db_impl_compaction_flush.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2264,21 +2264,27 @@ Status DBImpl::WaitForFlushMemTables(
int num = static_cast<int>(cfds.size());
// Wait until the compaction completes
InstrumentedMutexLock l(&mutex_);
Status s;
// If the caller is trying to resume from bg error, then
// error_handler_.IsDBStopped() is true.
while (resuming_from_bg_err || !error_handler_.IsDBStopped()) {
if (shutting_down_.load(std::memory_order_acquire)) {
return Status::ShutdownInProgress();
s = Status::ShutdownInProgress();
return s;
}
// If an error has occurred during resumption, then no need to wait.
// But flush operation may fail because of this error, so need to
// return the status.
if (!error_handler_.GetRecoveryError().ok()) {
s = error_handler_.GetRecoveryError();
break;
}
// If BGWorkStopped, which indicate that there is a BG error and
// 1) soft error but requires no BG work, 2) no in auto_recovery_
if (!resuming_from_bg_err && error_handler_.IsBGWorkStopped() &&
error_handler_.GetBGError().severity() < Status::Severity::kHardError) {
return error_handler_.GetBGError();
s = error_handler_.GetBGError();
return s;
}

// Number of column families that have been dropped.
Expand All @@ -2296,7 +2302,8 @@ Status DBImpl::WaitForFlushMemTables(
}
}
if (1 == num_dropped && 1 == num) {
return Status::ColumnFamilyDropped();
s = Status::ColumnFamilyDropped();
return s;
}
// Column families involved in this flush request have either been dropped
// or finished flush. Then it's time to finish waiting.
Expand All @@ -2305,7 +2312,6 @@ Status DBImpl::WaitForFlushMemTables(
}
bg_cv_.Wait();
}
Status s;
// If not resuming from bg error, and an error has caused the DB to stop,
// then report the bg error to caller.
if (!resuming_from_bg_err && error_handler_.IsDBStopped()) {
Expand Down

0 comments on commit 5f2b661

Please sign in to comment.