diff --git a/HISTORY.md b/HISTORY.md index fb40f71d6..823dffaba 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,8 @@ ## Unreleased ### Bug fixes * Fixed a bug after a `CompactRange()` with `CompactRangeOptions::change_level` set fails due to a conflict in the level change step, which caused all subsequent calls to `CompactRange()` with `CompactRangeOptions::change_level` set to incorrectly fail with a `Status::NotSupported("another thread is refitting")` error. +* Fixed a bug that the bottom most level compaction could still be a trivial move even if `BottommostLevelCompaction.kForce` or `kForceOptimized` is set. + ### Public API Change * The methods to create and manage EncrypedEnv have been changed. The EncryptionProvider is now passed to NewEncryptedEnv as a shared pointer, rather than a raw pointer. Comparably, the CTREncryptedProvider now takes a shared pointer, rather than a reference, to a BlockCipher. CreateFromString methods have been added to BlockCipher and EncryptionProvider to provide a single API by which different ciphers and providers can be created, respectively. * The internal classes (CTREncryptionProvider, ROT13BlockCipher, CTRCipherStream) associated with the EncryptedEnv have been moved out of the public API. To create a CTREncryptionProvider, one can either use EncryptionProvider::NewCTRProvider, or EncryptionProvider::CreateFromString("CTR"). To create a new ROT13BlockCipher, one can either use BlockCipher::NewROT13Cipher or BlockCipher::CreateFromString("ROT13"). diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index edb49db71..aac39d980 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -48,6 +48,18 @@ class DBCompactionTestWithParam bool exclusive_manual_compaction_; }; +class DBCompactionTestWithBottommostParam + : public DBTestBase, + public testing::WithParamInterface { + public: + DBCompactionTestWithBottommostParam() + : DBTestBase("/db_compaction_test", /*env_do_fsync=*/true) { + bottommost_level_compaction_ = GetParam(); + } + + BottommostLevelCompaction bottommost_level_compaction_; +}; + class DBCompactionDirectIOTest : public DBCompactionTest, public ::testing::WithParamInterface { public: @@ -5455,6 +5467,47 @@ TEST_P(DBCompactionTestWithParam, } } +TEST_P(DBCompactionTestWithBottommostParam, SequenceKeysManualCompaction) { + constexpr int kSstNum = 10; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + DestroyAndReopen(options); + + // Generate some sst files on level 0 with sequence keys (no overlap) + for (int i = 0; i < kSstNum; i++) { + for (int j = 1; j < UCHAR_MAX; j++) { + auto key = std::string(kSstNum, '\0'); + key[kSstNum - i] += static_cast(j); + Put(key, std::string(i % 1000, 'A')); + } + ASSERT_OK(Flush()); + } + ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable()); + + ASSERT_EQ(ToString(kSstNum), FilesPerLevel(0)); + + auto cro = CompactRangeOptions(); + cro.bottommost_level_compaction = bottommost_level_compaction_; + db_->CompactRange(cro, nullptr, nullptr); + if (bottommost_level_compaction_ == BottommostLevelCompaction::kForce || + bottommost_level_compaction_ == + BottommostLevelCompaction::kForceOptimized) { + // Real compaction to compact all sst files from level 0 to 1 file on level + // 1 + ASSERT_EQ("0,1", FilesPerLevel(0)); + } else { + // Just trivial move from level 0 -> 1 + ASSERT_EQ("0," + ToString(kSstNum), FilesPerLevel(0)); + } +} + +INSTANTIATE_TEST_CASE_P( + DBCompactionTestWithBottommostParam, DBCompactionTestWithBottommostParam, + ::testing::Values(BottommostLevelCompaction::kSkip, + BottommostLevelCompaction::kIfHaveCompactionFilter, + BottommostLevelCompaction::kForce, + BottommostLevelCompaction::kForceOptimized)); + TEST_F(DBCompactionTest, UpdateLevelSubCompactionTest) { Options options = CurrentOptions(); options.max_subcompactions = 10; diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index 50b149e73..0ea4d7d0d 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -873,6 +873,7 @@ Status DBImpl::CompactRange(const CompactRangeOptions& options, int output_level; for (int level = first_overlapped_level; level <= max_overlapped_level; level++) { + bool disallow_trivial_move = false; // in case the compaction is universal or if we're compacting the // bottom-most level, the output level will be the same as input one. // level 0 can never be the bottommost level (i.e. if all files are in @@ -905,9 +906,19 @@ Status DBImpl::CompactRange(const CompactRangeOptions& options, level == 0) { output_level = ColumnFamilyData::kCompactToBaseLevel; } + // if it's a BottommostLevel compaction and `kForce*` compaction is + // set, disallow trivial move + if (level == max_overlapped_level && + (options.bottommost_level_compaction == + BottommostLevelCompaction::kForce || + options.bottommost_level_compaction == + BottommostLevelCompaction::kForceOptimized)) { + disallow_trivial_move = true; + } } s = RunManualCompaction(cfd, level, output_level, options, begin, end, - exclusive, false, max_file_num_to_ignore); + exclusive, disallow_trivial_move, + max_file_num_to_ignore); if (!s.ok()) { break; }