diff --git a/src/leveldb/db/autocompact_test.cc b/src/leveldb/db/autocompact_test.cc index e6c97a05a6b..f6e714c6524 100644 --- a/src/leveldb/db/autocompact_test.cc +++ b/src/leveldb/db/autocompact_test.cc @@ -54,8 +54,8 @@ static const int kValueSize = 200 * 1024; static const int kTotalSize = 100 * 1024 * 1024; static const int kCount = kTotalSize / kValueSize; -// Read through the first n keys repeatedly and check that they get -// compacted (verified by checking the size of the key space). +// Read through the first n keys repeatedly and check that reads do NOT +// trigger compaction (seek compaction is disabled in this fork). void AutoCompactTest::DoReads(int n) { std::string value(kValueSize, 'x'); DBImpl* dbi = reinterpret_cast(db_); @@ -76,25 +76,23 @@ void AutoCompactTest::DoReads(int n) { const int64_t initial_size = Size(Key(0), Key(n)); const int64_t initial_other_size = Size(Key(n), Key(kCount)); - // Read until size drops significantly. + // Read repeatedly. The size of the read range must NOT shrink: with + // seek compaction disabled, reads never schedule a compaction. std::string limit_key = Key(n); - for (int read = 0; true; read++) { - ASSERT_LT(read, 100) << "Taking too long to compact"; + for (int read = 0; read < 100; read++) { Iterator* iter = db_->NewIterator(ReadOptions()); for (iter->SeekToFirst(); iter->Valid() && iter->key().ToString() < limit_key; iter->Next()) { // Drop data } delete iter; - // Wait a little bit to allow any triggered compactions to complete. - Env::Default()->SleepForMicroseconds(1000000); uint64_t size = Size(Key(0), Key(n)); fprintf(stderr, "iter %3d => %7.3f MB [other %7.3f MB]\n", read + 1, size / 1048576.0, Size(Key(n), Key(kCount)) / 1048576.0); - if (size <= initial_size / 10) { - break; - } } + // Give any background work a chance to run, even though none should. + Env::Default()->SleepForMicroseconds(1000000); + ASSERT_EQ(Size(Key(0), Key(n)), static_cast(initial_size)); // Verify that the size of the key space not touched by the reads // is pretty much unchanged. diff --git a/src/leveldb/db/db_test.cc b/src/leveldb/db/db_test.cc index 5ed9c1d75eb..48cc9373728 100644 --- a/src/leveldb/db/db_test.cc +++ b/src/leveldb/db/db_test.cc @@ -735,15 +735,14 @@ TEST(DBTest, GetPicksCorrectFile) { } while (ChangeOptions()); } -TEST(DBTest, GetEncountersEmptyLevel) { +TEST(DBTest, GetDoesNotTriggerSeekCompaction) { do { // Arrange for the following to happen: // * sstable A in level 0 // * nothing in level 1 // * sstable B in level 2 - // Then do enough Get() calls to arrange for an automatic compaction - // of sstable A. A bug would cause the compaction to be marked as - // occurring at level 1 (instead of the correct level 0). + // Seek compaction is disabled in this fork, so repeated reads must + // not change the level layout. A manual compaction must still work. // Step 1: First place sstables in levels 0 and 2 int compaction_count = 0; @@ -761,14 +760,17 @@ TEST(DBTest, GetEncountersEmptyLevel) { ASSERT_EQ(NumTableFilesAtLevel(1), 0); ASSERT_EQ(NumTableFilesAtLevel(2), 1); - // Step 3: read a bunch of times + // Step 3: many read misses must not schedule any compaction. for (int i = 0; i < 1000; i++) { ASSERT_EQ("NOT_FOUND", Get("missing")); } - - // Step 4: Wait for compaction to finish DelayMilliseconds(1000); + ASSERT_EQ(NumTableFilesAtLevel(0), 1); + ASSERT_EQ(NumTableFilesAtLevel(1), 0); + ASSERT_EQ(NumTableFilesAtLevel(2), 1); + // Step 4: a manual compaction still moves the L0 file down. + dbfull()->TEST_CompactRange(0, nullptr, nullptr); ASSERT_EQ(NumTableFilesAtLevel(0), 0); } while (ChangeOptions()); } diff --git a/src/leveldb/db/version_set.cc b/src/leveldb/db/version_set.cc index cd07346ea8a..8dc73295b84 100644 --- a/src/leveldb/db/version_set.cc +++ b/src/leveldb/db/version_set.cc @@ -400,16 +400,11 @@ Status Version::Get(const ReadOptions& options, const LookupKey& k, return state.found ? state.s : Status::NotFound(Slice()); } -bool Version::UpdateStats(const GetStats& stats) { - FileMetaData* f = stats.seek_file; - if (f != nullptr) { - f->allowed_seeks--; - if (f->allowed_seeks <= 0 && file_to_compact_ == nullptr) { - file_to_compact_ = f; - file_to_compact_level_ = stats.seek_file_level; - return true; - } - } +bool Version::UpdateStats(const GetStats& /*stats*/) { + // Disable automatic compactions triggered by read seek counters. + // The heuristic was tuned for expensive random seeks and can create + // severe write amplification on large random-key databases. + // Size and manual compactions still run. return false; } @@ -661,6 +656,8 @@ class VersionSet::Builder { // same as the compaction of 40KB of data. We are a little // conservative and allow approximately one seek for every 16KB // of data before triggering a compaction. + // + // Note: seek compactions are disabled. See Version::UpdateStats. f->allowed_seeks = static_cast((f->file_size / 16384U)); if (f->allowed_seeks < 100) f->allowed_seeks = 100;