From df082c8d1ddf5a90b195941064b56e853f104ff0 Mon Sep 17 00:00:00 2001 From: Changyu Bi <102700264+cbi42@users.noreply.github.com> Date: Wed, 5 Jul 2023 14:40:45 -0700 Subject: [PATCH] Deprecate option `periodic_compaction_seconds` for FIFO compaction (#11550) Summary: both options `ttl` and `periodic_compaction_seconds` have the same meaning for FIFO compaction, which is redundant and can be confusing to use. For example, setting TTL to 0 does not disable TTL: user needs to also set periodic_compaction_seconds to 0. Another example is that dynamically setting `periodic_compaction_seconds` (surprisingly) has no effect on TTL compaction. This is because FIFO compaction picker internally only looks at value of `ttl`. The value of `ttl` is in `SanitizeOptions()` which take into account the value of `periodic_compaction_seconds`, but dynamically setting an option does not invoke this method. This PR clarifies the usage of both options for FIFO compaction: only `ttl` should be used, `periodic_compaction_seconds` will not have any effect on FIFO compaction. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11550 Test Plan: - updated existing unit test `DBOptionsTest.SanitizeFIFOPeriodicCompaction` - checked existing values of both options in feature matrix: https://fburl.com/daiquery/xxd0gs9w. All current uses cases either have `periodic_compaction_seconds = 0` or have `periodic_compaction_seconds > ttl`, so should not cause change of behavior. Reviewed By: ajkr Differential Revision: D46902959 Pulled By: cbi42 fbshipit-source-id: a9ede235b276783b4906aaec443551fa62ceff4c --- db/column_family.cc | 20 +++++------ db/db_options_test.cc | 15 +++++---- include/rocksdb/advanced_options.h | 33 ++++++++----------- .../fifo_ttl_periodic_compaction.md | 1 + 4 files changed, 30 insertions(+), 39 deletions(-) create mode 100644 unreleased_history/behavior_changes/fifo_ttl_periodic_compaction.md diff --git a/db/column_family.cc b/db/column_family.cc index 42e13a13a..23349ba58 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -382,8 +382,8 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, const uint64_t kAdjustedTtl = 30 * 24 * 60 * 60; if (result.ttl == kDefaultTtl) { - if (is_block_based_table && - result.compaction_style != kCompactionStyleFIFO) { + if (is_block_based_table) { + // For FIFO, max_open_files is checked in ValidateOptions(). result.ttl = kAdjustedTtl; } else { result.ttl = 0; @@ -403,16 +403,12 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, result.periodic_compaction_seconds = kAdjustedPeriodicCompSecs; } } else { - // result.compaction_style == kCompactionStyleFIFO - if (result.ttl == 0) { - if (is_block_based_table) { - if (result.periodic_compaction_seconds == kDefaultPeriodicCompSecs) { - result.periodic_compaction_seconds = kAdjustedPeriodicCompSecs; - } - result.ttl = result.periodic_compaction_seconds; - } - } else if (result.periodic_compaction_seconds != 0) { - result.ttl = std::min(result.ttl, result.periodic_compaction_seconds); + if (result.periodic_compaction_seconds != kDefaultPeriodicCompSecs && + result.periodic_compaction_seconds > 0) { + ROCKS_LOG_WARN( + db_options.info_log.get(), + "periodic_compaction_seconds does not support FIFO compaction. You" + "may want to set option TTL instead."); } } diff --git a/db/db_options_test.cc b/db/db_options_test.cc index 533103f3c..d64d0eae5 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -880,9 +880,13 @@ TEST_F(DBOptionsTest, SanitizeFIFOPeriodicCompaction) { Options options; options.compaction_style = kCompactionStyleFIFO; options.env = CurrentOptions().env; + // Default value allows RocksDB to set ttl to 30 days. + ASSERT_EQ(30 * 24 * 60 * 60, dbfull()->GetOptions().ttl); + + // Disable options.ttl = 0; Reopen(options); - ASSERT_EQ(30 * 24 * 60 * 60, dbfull()->GetOptions().ttl); + ASSERT_EQ(0, dbfull()->GetOptions().ttl); options.ttl = 100; Reopen(options); @@ -892,15 +896,12 @@ TEST_F(DBOptionsTest, SanitizeFIFOPeriodicCompaction) { Reopen(options); ASSERT_EQ(100 * 24 * 60 * 60, dbfull()->GetOptions().ttl); - options.ttl = 200; - options.periodic_compaction_seconds = 300; - Reopen(options); - ASSERT_EQ(200, dbfull()->GetOptions().ttl); - + // periodic_compaction_seconds should have no effect + // on FIFO compaction. options.ttl = 500; options.periodic_compaction_seconds = 300; Reopen(options); - ASSERT_EQ(300, dbfull()->GetOptions().ttl); + ASSERT_EQ(500, dbfull()->GetOptions().ttl); } TEST_F(DBOptionsTest, SetFIFOCompactionOptions) { diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index 3af820b66..df7bb4e32 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -858,24 +858,23 @@ struct AdvancedColumnFamilyOptions { // Dynamically changeable through SetOptions() API bool report_bg_io_stats = false; - // Files containing updates older than TTL will go through the compaction - // process. This usually happens in a cascading way so that those entries - // will be compacted to bottommost level/file. - // The feature is used to remove stale entries that have been deleted or - // updated from the file system. - // Pre-req: This needs max_open_files to be set to -1. - // In Level: Non-bottom-level files older than TTL will go through the - // compaction process. - // In FIFO: Files older than TTL will be deleted. + // This option has different meanings for different compaction styles: + // + // Leveled: Non-bottom-level files with all keys older than TTL will go + // through the compaction process. This usually happens in a cascading + // way so that those entries will be compacted to bottommost level/file. + // The feature is used to remove stale entries that have been deleted or + // updated from the file system. + // + // FIFO: Files with all keys older than TTL will be deleted. TTL is only + // supported if option max_open_files is set to -1. + // // unit: seconds. Ex: 1 day = 1 * 24 * 60 * 60 - // In FIFO, this option will have the same meaning as - // periodic_compaction_seconds. Whichever stricter will be used. // 0 means disabling. // UINT64_MAX - 1 (0xfffffffffffffffe) is special flag to allow RocksDB to // pick default. // - // Default: 30 days for leveled compaction + block based table. disable - // otherwise. + // Default: 30 days if using block based table. 0 (disable) otherwise. // // Dynamically changeable through SetOptions() API uint64_t ttl = 0xfffffffffffffffe; @@ -891,12 +890,9 @@ struct AdvancedColumnFamilyOptions { // age is based on the file's last modified time (given by the underlying // Env). // - // Supported in all compaction styles. + // Supported in leveled and universal compaction. // In Universal compaction, rocksdb will try to do a full compaction when // possible, see more in UniversalCompactionBuilder::PickPeriodicCompaction(). - // In FIFO compaction, this option has the same meaning as TTL and whichever - // stricter will be used. - // Pre-req: max_open_file == -1. // unit: seconds. Ex: 7 days = 7 * 24 * 60 * 60 // // Values: @@ -905,9 +901,6 @@ struct AdvancedColumnFamilyOptions { // 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. - // In FIFO compaction, since the option has the same meaning as ttl, - // when this value is left default, and ttl is left to 0, 30 days will be - // used. Otherwise, min(ttl, periodic_compaction_seconds) will be used. // // Default: UINT64_MAX - 1 (allow RocksDB to auto-tune) // diff --git a/unreleased_history/behavior_changes/fifo_ttl_periodic_compaction.md b/unreleased_history/behavior_changes/fifo_ttl_periodic_compaction.md new file mode 100644 index 000000000..6297ccc91 --- /dev/null +++ b/unreleased_history/behavior_changes/fifo_ttl_periodic_compaction.md @@ -0,0 +1 @@ +Option `periodic_compaction_seconds` no longer supports FIFO compaction: setting it has no effect on FIFO compactions. FIFO compaction users should only set option `ttl` instead. \ No newline at end of file