mirror of
https://github.com/facebook/rocksdb.git
synced 2024-11-29 18:33:58 +00:00
Fix an issue of manual / auto compaction data race
Summary: A data race between a manual and an auto compaction can cause a scheduled automatic compaction to be cancelled and never rescheduled again. This may cause a condition of hanging forever. Fix this by always making sure the cancelled compaction is put back to the compaction queue. Closes https://github.com/facebook/rocksdb/pull/2238 Differential Revision: D4984591 Pulled By: siying fbshipit-source-id: 3ab153886403c7b991896dcb2158b96cac12f227
This commit is contained in:
parent
6798d1f3be
commit
af6fe69e4c
|
@ -1852,6 +1852,47 @@ TEST_F(DBCompactionTest, L0_CompactionBug_Issue44_b) {
|
|||
} while (ChangeCompactOptions());
|
||||
}
|
||||
|
||||
TEST_F(DBCompactionTest, ManualAutoRace) {
|
||||
CreateAndReopenWithCF({"pikachu"}, CurrentOptions());
|
||||
rocksdb::SyncPoint::GetInstance()->LoadDependency(
|
||||
{{"DBImpl::BGWorkCompaction", "DBCompactionTest::ManualAutoRace:1"},
|
||||
{"DBImpl::RunManualCompaction:WaitScheduled",
|
||||
"BackgroundCallCompaction:0"}});
|
||||
|
||||
rocksdb::SyncPoint::GetInstance()->EnableProcessing();
|
||||
|
||||
Put(1, "foo", "");
|
||||
Put(1, "bar", "");
|
||||
Flush(1);
|
||||
Put(1, "foo", "");
|
||||
Put(1, "bar", "");
|
||||
// Generate four files in CF 0, which should trigger an auto compaction
|
||||
Put("foo", "");
|
||||
Put("bar", "");
|
||||
Flush();
|
||||
Put("foo", "");
|
||||
Put("bar", "");
|
||||
Flush();
|
||||
Put("foo", "");
|
||||
Put("bar", "");
|
||||
Flush();
|
||||
Put("foo", "");
|
||||
Put("bar", "");
|
||||
Flush();
|
||||
|
||||
// The auto compaction is scheduled but waited until here
|
||||
TEST_SYNC_POINT("DBCompactionTest::ManualAutoRace:1");
|
||||
// The auto compaction will wait until the the manual compaction is registerd
|
||||
// before processing so that it will be cancelled.
|
||||
dbfull()->CompactRange(CompactRangeOptions(), handles_[1], nullptr, nullptr);
|
||||
ASSERT_EQ("0,1", FilesPerLevel(1));
|
||||
|
||||
// Eventually the cancelled compaction will be rescheduled and executed.
|
||||
dbfull()->TEST_WaitForCompact();
|
||||
ASSERT_EQ("0,1", FilesPerLevel(0));
|
||||
rocksdb::SyncPoint::GetInstance()->DisableProcessing();
|
||||
}
|
||||
|
||||
TEST_P(DBCompactionTestWithParam, ManualCompaction) {
|
||||
Options options = CurrentOptions();
|
||||
options.max_subcompactions = max_subcompactions_;
|
||||
|
|
|
@ -828,6 +828,7 @@ Status DBImpl::RunManualCompaction(ColumnFamilyData* cfd, int input_level,
|
|||
TEST_SYNC_POINT_CALLBACK("DBImpl::RunManualCompaction:NotScheduled", &mutex_);
|
||||
if (exclusive) {
|
||||
while (bg_compaction_scheduled_ > 0) {
|
||||
TEST_SYNC_POINT("DBImpl::RunManualCompaction:WaitScheduled");
|
||||
ROCKS_LOG_INFO(
|
||||
immutable_db_options_.info_log,
|
||||
"[%s] Manual compaction waiting for all other scheduled background "
|
||||
|
@ -1394,6 +1395,16 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
|
|||
: m->manual_end->DebugString().c_str()));
|
||||
}
|
||||
} else if (!compaction_queue_.empty()) {
|
||||
if (HaveManualCompaction(compaction_queue_.front())) {
|
||||
// Can't compact right now, but try again later
|
||||
TEST_SYNC_POINT("DBImpl::BackgroundCompaction()::Conflict");
|
||||
|
||||
// Stay in the compaciton queue.
|
||||
unscheduled_compactions_++;
|
||||
|
||||
return Status::OK();
|
||||
}
|
||||
|
||||
// cfd is referenced here
|
||||
auto cfd = PopFirstFromCompactionQueue();
|
||||
// We unreference here because the following code will take a Ref() on
|
||||
|
@ -1408,12 +1419,6 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
|
|||
return Status::OK();
|
||||
}
|
||||
|
||||
if (HaveManualCompaction(cfd)) {
|
||||
// Can't compact right now, but try again later
|
||||
TEST_SYNC_POINT("DBImpl::BackgroundCompaction()::Conflict");
|
||||
return Status::OK();
|
||||
}
|
||||
|
||||
// Pick up latest mutable CF Options and use it throughout the
|
||||
// compaction job
|
||||
// Compaction makes a copy of the latest MutableCFOptions. It should be used
|
||||
|
|
Loading…
Reference in a new issue