Skip to content

Commit 073cbfc

Browse files
author
Siying Dong
committed
Enable background flush thread by default and fix issues related to it
Summary: Enable background flush thread in this patch and fix unit tests with: (1) After background flush, schedule a background compaction if condition satisfied; (2) Fix a bug that if universal compaction is enabled and number of levels are set to be 0, compaction will not be automatically triggered (3) Fix unit tests to wait for compaction to finish instead of flush, before checking the compaction results. Test Plan: pass all unit tests Reviewers: haobo, xjin, dhruba Reviewed By: haobo CC: leveldb Differential Revision: https://reviews.facebook.net/D13461
1 parent cb5b2ba commit 073cbfc

File tree

6 files changed

+28
-7
lines changed

6 files changed

+28
-7
lines changed

db/corruption_test.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ TEST(CorruptionTest, CompactionInputErrorParanoid) {
324324

325325
Build(10);
326326
dbi->TEST_FlushMemTable();
327+
dbi->TEST_WaitForCompact();
327328
ASSERT_EQ(1, Property("rocksdb.num-files-at-level0"));
328329

329330
Corrupt(kTableFile, 100, 1);

db/db_impl.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,24 +1436,25 @@ void DBImpl::BGWorkCompaction(void* db) {
14361436
reinterpret_cast<DBImpl*>(db)->BackgroundCallCompaction();
14371437
}
14381438

1439-
Status DBImpl::BackgroundFlush() {
1439+
Status DBImpl::BackgroundFlush(bool* madeProgress) {
14401440
Status stat;
14411441
while (stat.ok() &&
14421442
imm_.IsFlushPending(options_.min_write_buffer_number_to_merge)) {
14431443
Log(options_.info_log,
14441444
"BackgroundCallFlush doing FlushMemTableToOutputFile, flush slots available %d",
14451445
options_.max_background_flushes - bg_flush_scheduled_);
1446-
stat = FlushMemTableToOutputFile();
1446+
stat = FlushMemTableToOutputFile(madeProgress);
14471447
}
14481448
return stat;
14491449
}
14501450

14511451
void DBImpl::BackgroundCallFlush() {
1452+
bool madeProgress = false;
14521453
assert(bg_flush_scheduled_);
14531454
MutexLock l(&mutex_);
14541455

14551456
if (!shutting_down_.Acquire_Load()) {
1456-
Status s = BackgroundFlush();
1457+
Status s = BackgroundFlush(&madeProgress);
14571458
if (!s.ok()) {
14581459
// Wait a little bit before retrying background compaction in
14591460
// case this is an environmental problem and we do not want to
@@ -1469,7 +1470,9 @@ void DBImpl::BackgroundCallFlush() {
14691470
}
14701471

14711472
bg_flush_scheduled_--;
1472-
1473+
if (madeProgress) {
1474+
MaybeScheduleFlushOrCompaction();
1475+
}
14731476
bg_cv_.SignalAll();
14741477
}
14751478

db/db_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class DBImpl : public DB {
186186
void BackgroundCallCompaction();
187187
void BackgroundCallFlush();
188188
Status BackgroundCompaction(bool* madeProgress,DeletionState& deletion_state);
189-
Status BackgroundFlush();
189+
Status BackgroundFlush(bool* madeProgress);
190190
void CleanupCompaction(CompactionState* compact);
191191
Status DoCompactionWork(CompactionState* compact);
192192

db/db_test.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1850,6 +1850,8 @@ TEST(DBTest, UniversalCompactionSizeAmplification) {
18501850
// but will instead trigger size amplification.
18511851
dbfull()->Flush(FlushOptions());
18521852

1853+
dbfull()->TEST_WaitForCompact();
1854+
18531855
// Verify that size amplification did occur
18541856
ASSERT_EQ(NumTableFilesAtLevel(0), 1);
18551857
}

db/version_set.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1645,7 +1645,12 @@ void VersionSet::Finalize(Version* v,
16451645
double max_score = 0;
16461646
int max_score_level = 0;
16471647

1648-
for (int level = 0; level < NumberLevels()-1; level++) {
1648+
int num_levels_to_check =
1649+
(options_->compaction_style != kCompactionStyleUniversal) ?
1650+
NumberLevels() - 1 : 1;
1651+
1652+
for (int level = 0; level < num_levels_to_check; level++) {
1653+
16491654
double score;
16501655
if (level == 0) {
16511656
// We treat level-0 specially by bounding the number of files

db/version_set.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,17 @@ class VersionSet {
312312
// Returns true iff some level needs a compaction because it has
313313
// exceeded its target size.
314314
bool NeedsSizeCompaction() const {
315-
for (int i = 0; i < NumberLevels()-1; i++) {
315+
// In universal compaction case, this check doesn't really
316+
// check the compaction condition, but checks num of files threshold
317+
// only. We are not going to miss any compaction opportunity
318+
// but it's likely that more compactions are scheduled but
319+
// ending up with nothing to do. We can improve it later.
320+
// TODO: improve this function to be accurate for universal
321+
// compactions.
322+
int num_levels_to_check =
323+
(options_->compaction_style != kCompactionStyleUniversal) ?
324+
NumberLevels() - 1 : 1;
325+
for (int i = 0; i < num_levels_to_check; i++) {
316326
if (current_->compaction_score_[i] >= 1) {
317327
return true;
318328
}

0 commit comments

Comments
 (0)