From c2b0bb579a67ba72d3488828c2dc3d1b03937be0 Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Sun, 29 Sep 2019 11:30:58 -0700 Subject: [PATCH 1/6] Enable Periodic Compactions automatically if Compaction Filter is used --- db/column_family.cc | 12 +++++--- db/db_compaction_test.cc | 66 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 16688d6cee2..adefd29663a 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -342,6 +342,13 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, result.max_compaction_bytes = result.target_file_size_base * 25; } + if (result.compaction_style == kCompactionStyleLevel && + (result.compaction_filter != nullptr || + result.compaction_filter_factory != nullptr) && + result.periodic_compaction_seconds == 0) { + result.periodic_compaction_seconds = 30 * 24 * 60 * 60; + } + return result; } @@ -1181,11 +1188,6 @@ Status ColumnFamilyData::ValidateOptions( } if (cf_options.periodic_compaction_seconds > 0) { - if (db_options.max_open_files != -1) { - return Status::NotSupported( - "Periodic Compaction is only supported when files are always " - "kept open (set max_open_files = -1). "); - } if (cf_options.table_factory->Name() != BlockBasedTableFactory().Name()) { return Status::NotSupported( "Periodic Compaction is only supported in " diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index dad19921c12..94df522871d 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -3748,6 +3748,72 @@ TEST_F(DBCompactionTest, LevelPeriodicAndTtlCompaction) { rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } +TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { + const int kNumKeysPerFile = 32; + const int kNumLevelFiles = 2; + const int kValueSize = 100; + + Random rnd(301); + + Options options = CurrentOptions(); + options.compaction_filter = test::RandomCompactionFilter(&rnd); + env_->time_elapse_only_sleep_ = false; + options.env = env_; + env_->addon_time_.store(0); + + for (int comp_filter_type = 0; comp_filter_type < 2; comp_filter_type++) { + // Assert that periodic compactions are not enabled. + ASSERT_EQ(0, options.periodic_compaction_seconds); + + if (comp_filter_type == 0) { + options.compaction_filter = test::RandomCompactionFilter(&rnd); + } else if (comp_filter_type == 1) { + options.compaction_filter_factory.reset( + test::RandomCompactionFilterFactory(&rnd)); + } + DestroyAndReopen(options); + + // periodic_compaction_seconds should be set to the sanitized value since + // a compaction filter or a compaction filter factory is used. + ASSERT_EQ(30 * 24 * 60 * 60, + dbfull()->GetOptions().periodic_compaction_seconds); + + int periodic_compactions = 0; + rocksdb::SyncPoint::GetInstance()->SetCallBack( + "LevelCompactionPicker::PickCompaction:Return", [&](void* arg) { + Compaction* compaction = reinterpret_cast(arg); + auto compaction_reason = compaction->compaction_reason(); + if (compaction_reason == CompactionReason::kPeriodicCompaction) { + periodic_compactions++; + } + }); + rocksdb::SyncPoint::GetInstance()->EnableProcessing(); + + for (int i = 0; i < kNumLevelFiles; ++i) { + for (int j = 0; j < kNumKeysPerFile; ++j) { + ASSERT_OK( + Put(Key(i * kNumKeysPerFile + j), RandomString(&rnd, kValueSize))); + } + Flush(); + } + dbfull()->TEST_WaitForCompact(); + + ASSERT_EQ("2", FilesPerLevel()); + ASSERT_EQ(0, periodic_compactions); + + // Add 50 hours and do a write + env_->addon_time_.fetch_add(31 * 24 * 60 * 60); + ASSERT_OK(Put("a", "1")); + Flush(); + dbfull()->TEST_WaitForCompact(); + // Assert that the files stay in the same level + ASSERT_EQ("3", FilesPerLevel()); + // The two old files go through the periodic compaction process + ASSERT_EQ(2, periodic_compactions); + + rocksdb::SyncPoint::GetInstance()->DisableProcessing(); + } +} TEST_F(DBCompactionTest, CompactRangeDelayedByL0FileCount) { // Verify that, when `CompactRangeOptions::allow_write_stall == false`, manual From dfc9485a4a330635e6156b5c88378b8b43f95edb Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Sun, 29 Sep 2019 18:16:32 -0700 Subject: [PATCH 2/6] Fix memory leak in test --- db/db_compaction_test.cc | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index 94df522871d..6b9586af6a7 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -3749,6 +3749,17 @@ TEST_F(DBCompactionTest, LevelPeriodicAndTtlCompaction) { } TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { + class TestCompactionFilter : public CompactionFilter { + const char* Name() const override { return "TestCompactionFilter"; } + }; + class TestCompactionFilterFactory : public CompactionFilterFactory { + const char* Name() const override { return "TestCompactionFilterFactory"; } + std::unique_ptr CreateCompactionFilter( + const CompactionFilter::Context& /*context*/) override { + return std::unique_ptr(new TestCompactionFilter()); + } + }; + const int kNumKeysPerFile = 32; const int kNumLevelFiles = 2; const int kValueSize = 100; @@ -3756,20 +3767,28 @@ TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { Random rnd(301); Options options = CurrentOptions(); - options.compaction_filter = test::RandomCompactionFilter(&rnd); + TestCompactionFilter test_compaction_filter; env_->time_elapse_only_sleep_ = false; options.env = env_; env_->addon_time_.store(0); - for (int comp_filter_type = 0; comp_filter_type < 2; comp_filter_type++) { + enum COMPACTION_FILTER_TYPE { + USE_COMPACTION_FILTER, + USE_COMPCTION_FILTER_FACTORY + }; + + for (COMPACTION_FILTER_TYPE comp_filter_type : + {USE_COMPACTION_FILTER, USE_COMPCTION_FILTER_FACTORY}) { // Assert that periodic compactions are not enabled. ASSERT_EQ(0, options.periodic_compaction_seconds); - if (comp_filter_type == 0) { - options.compaction_filter = test::RandomCompactionFilter(&rnd); - } else if (comp_filter_type == 1) { + if (comp_filter_type == USE_COMPACTION_FILTER) { + options.compaction_filter = &test_compaction_filter; + options.compaction_filter_factory.reset(); + } else if (comp_filter_type == USE_COMPCTION_FILTER_FACTORY) { + options.compaction_filter = nullptr; options.compaction_filter_factory.reset( - test::RandomCompactionFilterFactory(&rnd)); + new TestCompactionFilterFactory()); } DestroyAndReopen(options); From 98916028803e4990bf1d2382bfd5e5287b24c167 Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Sun, 29 Sep 2019 18:19:58 -0700 Subject: [PATCH 3/6] Fix comment --- db/db_compaction_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index 6b9586af6a7..ca39a883514 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -3792,7 +3792,7 @@ TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { } DestroyAndReopen(options); - // periodic_compaction_seconds should be set to the sanitized value since + // periodic_compaction_seconds should be set to the sanitized value when // a compaction filter or a compaction filter factory is used. ASSERT_EQ(30 * 24 * 60 * 60, dbfull()->GetOptions().periodic_compaction_seconds); @@ -3820,7 +3820,7 @@ TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { ASSERT_EQ("2", FilesPerLevel()); ASSERT_EQ(0, periodic_compactions); - // Add 50 hours and do a write + // Add 31 days and do a write env_->addon_time_.fetch_add(31 * 24 * 60 * 60); ASSERT_OK(Put("a", "1")); Flush(); From c1760c075b37d03a248cdfe724428ce7b2e01946 Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Mon, 28 Oct 2019 16:29:45 -0700 Subject: [PATCH 4/6] Change periodic_compaction_seconds to UINT64_MAX --- db/column_family.cc | 9 +++++++-- db/db_compaction_test.cc | 2 +- db/version_set.cc | 13 +++++++++++-- include/rocksdb/advanced_options.h | 12 ++++++++++-- utilities/blob_db/blob_db_impl.cc | 3 +++ 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index adefd29663a..f0360eefe96 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -30,6 +30,7 @@ #include "memtable/hash_skiplist_rep.h" #include "monitoring/thread_status_util.h" #include "options/options_helper.h" +#include "port/port.h" #include "table/block_based/block_based_table_factory.h" #include "table/merging_iterator.h" #include "util/autovector.h" @@ -342,10 +343,13 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, result.max_compaction_bytes = result.target_file_size_base * 25; } + // Turn on periodic compactions and set them to occur once every 30 days if + // compaction filters are used and periodic_compaction_seconds is set to the + // default value. if (result.compaction_style == kCompactionStyleLevel && (result.compaction_filter != nullptr || result.compaction_filter_factory != nullptr) && - result.periodic_compaction_seconds == 0) { + result.periodic_compaction_seconds == port::kMaxUint64) { result.periodic_compaction_seconds = 30 * 24 * 60 * 60; } @@ -1187,7 +1191,8 @@ Status ColumnFamilyData::ValidateOptions( } } - if (cf_options.periodic_compaction_seconds > 0) { + if (cf_options.periodic_compaction_seconds > 0 && + cf_options.periodic_compaction_seconds < port::kMaxUint64) { if (cf_options.table_factory->Name() != BlockBasedTableFactory().Name()) { return Status::NotSupported( "Periodic Compaction is only supported in " diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index ca39a883514..45c140ab572 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -3780,7 +3780,7 @@ TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { for (COMPACTION_FILTER_TYPE comp_filter_type : {USE_COMPACTION_FILTER, USE_COMPCTION_FILTER_FACTORY}) { // Assert that periodic compactions are not enabled. - ASSERT_EQ(0, options.periodic_compaction_seconds); + ASSERT_EQ(port::kMaxUint64, options.periodic_compaction_seconds); if (comp_filter_type == USE_COMPACTION_FILTER) { options.compaction_filter = &test_compaction_filter; diff --git a/db/version_set.cc b/db/version_set.cc index 11264205a29..61d140a6fb6 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -2445,7 +2445,8 @@ void VersionStorageInfo::ComputeCompactionScore( if (mutable_cf_options.ttl > 0) { ComputeExpiredTtlFiles(immutable_cf_options, mutable_cf_options.ttl); } - if (mutable_cf_options.periodic_compaction_seconds > 0) { + if (mutable_cf_options.periodic_compaction_seconds > 0 && + mutable_cf_options.periodic_compaction_seconds < port::kMaxUint64) { ComputeFilesMarkedForPeriodicCompaction( immutable_cf_options, mutable_cf_options.periodic_compaction_seconds); } @@ -2505,7 +2506,8 @@ void VersionStorageInfo::ComputeExpiredTtlFiles( void VersionStorageInfo::ComputeFilesMarkedForPeriodicCompaction( const ImmutableCFOptions& ioptions, const uint64_t periodic_compaction_seconds) { - assert(periodic_compaction_seconds > 0); + assert(periodic_compaction_seconds > 0 && + periodic_compaction_seconds < port::kMaxUint64); files_marked_for_periodic_compaction_.clear(); @@ -2515,6 +2517,13 @@ void VersionStorageInfo::ComputeFilesMarkedForPeriodicCompaction( return; } const uint64_t current_time = static_cast(temp_current_time); + + assert(periodic_compaction_seconds <= current_time); + // Disable periodic compaction if periodic_compaction_seconds > current_time. + // This also help handle the underflow case. + if (periodic_compaction_seconds > current_time) { + return; + } const uint64_t allowed_time_limit = current_time - periodic_compaction_seconds; diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index 2964491f7ee..77c55d977ef 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -670,10 +670,18 @@ struct AdvancedColumnFamilyOptions { // Only supported in Level compaction. // Pre-req: max_open_file == -1. // unit: seconds. Ex: 7 days = 7 * 24 * 60 * 60 - // Default: 0 (disabled) + // + // Values: + // 0: Turn off Periodic compactions. + // UINT64_MAX (i.e 0xffffffffffffffff): Let RocksDB control this feature + // as needed. For now, RocksDB will change this value to 30 days + // (i.e 30 * 24 * 60 * 60) so that every file goes through the compaction + // process at least once every 30 days if not compacted sooner. + // + // Default: UINT64_MAX (allow RocksDB to auto-tune) // // Dynamically changeable through SetOptions() API - uint64_t periodic_compaction_seconds = 0; + uint64_t periodic_compaction_seconds = 0xffffffffffffffff; // If this option is set then 1 in N blocks are compressed // using a fast (lz4) and slow (zstd) compression algorithm. diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index b7338f2305f..d3a4d8e4aec 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -137,6 +137,9 @@ Status BlobDBImpl::Open(std::vector* handles) { cf_options_.compaction_filter_factory != nullptr) { return Status::NotSupported("Blob DB doesn't support compaction filter."); } + // BlobDB does not support Periodic Compactions. So disable periodic + // compactions irrespective of the user set value. + cf_options_.periodic_compaction_seconds = 0; Status s; From c926bc907c0b8048a4acc1e9d0e0b5951090646c Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Tue, 29 Oct 2019 09:32:37 -0700 Subject: [PATCH 5/6] Update HISTORY.md --- HISTORY.md | 1 + 1 file changed, 1 insertion(+) diff --git a/HISTORY.md b/HISTORY.md index 1ff833b6717..63254df953e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,7 @@ # Rocksdb Change Log ## Unreleased ### Public API Change +* Changed the default value of periodic_compaction_seconds to `UINT64_MAX` which allows RocksDB to auto-tune periodic compaction scheduling. When using the default value, periodic compactions are now auto-enabled if a compaction filter is used. A value of `0` will turn off the feature completely. * Added an API GetCreationTimeOfOldestFile(uint64_t* creation_time) to get the file_creation_time of the oldest SST file in the DB. From 7fa0bebc03641cc39ff21236ed1bec8a1092e20b Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Tue, 29 Oct 2019 13:26:02 -0700 Subject: [PATCH 6/6] Change enum name according to the style guide --- db/db_compaction_test.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index 45c140ab572..bf301d9834a 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -3772,20 +3772,20 @@ TEST_F(DBCompactionTest, LevelPeriodicCompactionWithCompactionFilters) { options.env = env_; env_->addon_time_.store(0); - enum COMPACTION_FILTER_TYPE { - USE_COMPACTION_FILTER, - USE_COMPCTION_FILTER_FACTORY + enum CompactionFilterType { + kUseCompactionFilter, + kUseCompactionFilterFactory }; - for (COMPACTION_FILTER_TYPE comp_filter_type : - {USE_COMPACTION_FILTER, USE_COMPCTION_FILTER_FACTORY}) { + for (CompactionFilterType comp_filter_type : + {kUseCompactionFilter, kUseCompactionFilterFactory}) { // Assert that periodic compactions are not enabled. ASSERT_EQ(port::kMaxUint64, options.periodic_compaction_seconds); - if (comp_filter_type == USE_COMPACTION_FILTER) { + if (comp_filter_type == kUseCompactionFilter) { options.compaction_filter = &test_compaction_filter; options.compaction_filter_factory.reset(); - } else if (comp_filter_type == USE_COMPCTION_FILTER_FACTORY) { + } else if (comp_filter_type == kUseCompactionFilterFactory) { options.compaction_filter = nullptr; options.compaction_filter_factory.reset( new TestCompactionFilterFactory());