From a9c5109515e5a1670383658600e9d0e5590a420f Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Tue, 14 Jul 2015 13:07:02 +0200 Subject: [PATCH] Deprecate purge_redundant_kvs_while_flush Summary: This option is guarding the feature implemented 2 and a half years ago: D8991. The feature was enabled by default back then and has been running without issues. There is no reason why any client would turn this feature off. I found no reference in fbcode. Test Plan: none Reviewers: sdong, yhchiang, anthony, dhruba Reviewed By: dhruba Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D42063 --- HISTORY.md | 1 + db/builder.cc | 2 +- db/c.cc | 7 +++---- db/db_test.cc | 7 +------ include/rocksdb/options.h | 4 ++-- tools/db_stress.cc | 15 --------------- util/db_test_util.cc | 6 +----- util/db_test_util.h | 25 ++++++++++++------------- util/options.cc | 4 ---- util/options_helper.cc | 3 --- util/options_test.cc | 2 -- 11 files changed, 21 insertions(+), 55 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 1a31479e35..a0b60f6ec4 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -4,6 +4,7 @@ ### Public API changes * Deprecated WriteOptions::timeout_hint_us. We no longer support write timeout. If you really need this option, talk to us and we might consider returning it. +* Deprecated purge_redundant_kvs_while_flush option. ## 3.12.0 (7/2/2015) ### New Features diff --git a/db/builder.cc b/db/builder.cc index 9436534fcd..a04075ba62 100644 --- a/db/builder.cc +++ b/db/builder.cc @@ -64,7 +64,7 @@ Status BuildTable( // If the sequence number of the smallest entry in the memtable is // smaller than the most recent snapshot, then we do not trigger // removal of duplicate/deleted keys as part of this builder. - bool purge = ioptions.purge_redundant_kvs_while_flush; + bool purge = true; if (earliest_seqno_in_memtable <= newest_snapshot) { purge = false; } diff --git a/db/c.cc b/db/c.cc index c2b0550a71..3e958c7653 100644 --- a/db/c.cc +++ b/db/c.cc @@ -1674,10 +1674,9 @@ void rocksdb_options_set_manifest_preallocation_size( opt->rep.manifest_preallocation_size = v; } -void rocksdb_options_set_purge_redundant_kvs_while_flush( - rocksdb_options_t* opt, unsigned char v) { - opt->rep.purge_redundant_kvs_while_flush = v; -} +// noop +void rocksdb_options_set_purge_redundant_kvs_while_flush(rocksdb_options_t* opt, + unsigned char v) {} void rocksdb_options_set_allow_os_buffer(rocksdb_options_t* opt, unsigned char v) { diff --git a/db/db_test.cc b/db/db_test.cc index b2745b8b7d..9162e40952 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -5806,11 +5806,7 @@ TEST_F(DBTest, DeletionMarkers1) { Put(1, "foo", "v2"); ASSERT_EQ(AllEntriesFor("foo", 1), "[ v2, DEL, v1 ]"); ASSERT_OK(Flush(1)); // Moves to level last-2 - if (CurrentOptions().purge_redundant_kvs_while_flush) { - ASSERT_EQ(AllEntriesFor("foo", 1), "[ v2, v1 ]"); - } else { - ASSERT_EQ(AllEntriesFor("foo", 1), "[ v2, DEL, v1 ]"); - } + ASSERT_EQ(AllEntriesFor("foo", 1), "[ v2, v1 ]"); Slice z("z"); dbfull()->TEST_CompactRange(last - 2, nullptr, &z, handles_[1]); // DEL eliminated, but v1 remains because we aren't compacting that level @@ -6879,7 +6875,6 @@ TEST_F(DBTest, CompactOnFlush) { options_override.skip_policy = kSkipNoSnapshot; do { Options options = CurrentOptions(options_override); - options.purge_redundant_kvs_while_flush = true; options.disable_auto_compactions = true; CreateAndReopenWithCF({"pikachu"}, options); diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index 8cb0d1215f..902a435a05 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -540,8 +540,8 @@ struct ColumnFamilyOptions { // Dynamically changeable through SetOptions() API bool disable_auto_compactions; - // Purge duplicate/deleted keys when a memtable is flushed to storage. - // Default: true + // DEPREACTED + // Does not have any effect. bool purge_redundant_kvs_while_flush; // The compaction style. Default: kCompactionStyleLevel diff --git a/tools/db_stress.cc b/tools/db_stress.cc index 173b93371f..29cf488cfb 100644 --- a/tools/db_stress.cc +++ b/tools/db_stress.cc @@ -359,13 +359,6 @@ DEFINE_uint64(log2_keys_per_lock, 2, "Log2 of number of keys per lock"); static const bool FLAGS_log2_keys_per_lock_dummy __attribute__((unused)) = RegisterFlagValidator(&FLAGS_log2_keys_per_lock, &ValidateUint32Range); -DEFINE_int32(purge_redundant_percent, 50, - "Percentage of times we want to purge redundant keys in memory " - "before flushing"); -static const bool FLAGS_purge_redundant_percent_dummy __attribute__((unused)) = - RegisterFlagValidator(&FLAGS_purge_redundant_percent, - &ValidateInt32Percent); - DEFINE_bool(filter_deletes, false, "On true, deletes use KeyMayExist to drop" " the delete if key not present"); @@ -1812,8 +1805,6 @@ class StressTest { fprintf(stdout, "Num times DB reopens: %d\n", FLAGS_reopen); fprintf(stdout, "Batches/snapshots : %d\n", FLAGS_test_batches_snapshots); - fprintf(stdout, "Purge redundant %% : %d\n", - FLAGS_purge_redundant_percent); fprintf(stdout, "Deletes use filter : %d\n", FLAGS_filter_deletes); fprintf(stdout, "Do update in place : %d\n", @@ -1931,12 +1922,6 @@ class StressTest { #endif // ROCKSDB_LITE } - static Random purge_percent(1000); // no benefit from non-determinism here - if (static_cast(purge_percent.Uniform(100)) < - FLAGS_purge_redundant_percent - 1) { - options_.purge_redundant_kvs_while_flush = false; - } - if (FLAGS_use_merge) { options_.merge_operator = MergeOperators::CreatePutOperator(); } diff --git a/util/db_test_util.cc b/util/db_test_util.cc index efdf089f61..b38e979475 100644 --- a/util/db_test_util.cc +++ b/util/db_test_util.cc @@ -236,11 +236,7 @@ Options DBTestBase::CurrentOptions( break; case kManifestFileSize: options.max_manifest_file_size = 50; // 50 bytes - case kCompactOnFlush: - options.purge_redundant_kvs_while_flush = - !options.purge_redundant_kvs_while_flush; - break; - case kPerfOptions: + case kPerfOptions: options.soft_rate_limit = 2.0; options.delayed_write_rate = 8 * 1024 * 1024; // TODO(3.13) -- test more options diff --git a/util/db_test_util.h b/util/db_test_util.h index 440442f6e2..e2be831b83 100644 --- a/util/db_test_util.h +++ b/util/db_test_util.h @@ -399,19 +399,18 @@ class DBTestBase : public testing::Test { kDBLogDir = 14, kWalDirAndMmapReads = 15, kManifestFileSize = 16, - kCompactOnFlush = 17, - kPerfOptions = 18, - kDeletesFilterFirst = 19, - kHashSkipList = 20, - kUniversalCompaction = 21, - kUniversalCompactionMultiLevel = 22, - kCompressedBlockCache = 23, - kInfiniteMaxOpenFiles = 24, - kxxHashChecksum = 25, - kFIFOCompaction = 26, - kOptimizeFiltersForHits = 27, - kRowCache = 28, - kEnd = 29 + kPerfOptions = 17, + kDeletesFilterFirst = 18, + kHashSkipList = 19, + kUniversalCompaction = 20, + kUniversalCompactionMultiLevel = 21, + kCompressedBlockCache = 22, + kInfiniteMaxOpenFiles = 23, + kxxHashChecksum = 24, + kFIFOCompaction = 25, + kOptimizeFiltersForHits = 26, + kRowCache = 27, + kEnd = 28 }; int option_config_; diff --git a/util/options.cc b/util/options.cc index ce0f7572b3..56950197c2 100644 --- a/util/options.cc +++ b/util/options.cc @@ -407,8 +407,6 @@ void ColumnFamilyOptions::Dump(Logger* log) const { Warn(log, " Options.num_levels: %d", num_levels); Warn(log, " Options.min_write_buffer_number_to_merge: %d", min_write_buffer_number_to_merge); - Warn(log, " Options.purge_redundant_kvs_while_flush: %d", - purge_redundant_kvs_while_flush); Warn(log, " Options.max_write_buffer_number_to_maintain: %d", max_write_buffer_number_to_maintain); Warn(log, " Options.compression_opts.window_bits: %d", @@ -461,8 +459,6 @@ void ColumnFamilyOptions::Dump(Logger* log) const { rate_limit_delay_max_milliseconds); Warn(log, " Options.disable_auto_compactions: %d", disable_auto_compactions); - Warn(log, " Options.purge_redundant_kvs_while_flush: %d", - purge_redundant_kvs_while_flush); Warn(log, " Options.filter_deletes: %d", filter_deletes); Warn(log, " Options.verify_checksums_in_compaction: %d", diff --git a/util/options_helper.cc b/util/options_helper.cc index d6797a7ced..4eafd2affa 100644 --- a/util/options_helper.cc +++ b/util/options_helper.cc @@ -437,9 +437,6 @@ bool ParseColumnFamilyOption(const std::string& name, const std::string& value, } else if (name == "level_compaction_dynamic_level_bytes") { new_options->level_compaction_dynamic_level_bytes = ParseBoolean(name, value); - } else if (name == "purge_redundant_kvs_while_flush") { - new_options->purge_redundant_kvs_while_flush = - ParseBoolean(name, value); } else if (name == "compaction_style") { new_options->compaction_style = ParseCompactionStyle(value); } else if (name == "compaction_options_universal") { diff --git a/util/options_test.cc b/util/options_test.cc index 6a0e96ae6e..33867f1f2d 100644 --- a/util/options_test.cc +++ b/util/options_test.cc @@ -124,7 +124,6 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) { {"hard_rate_limit", "2.1"}, {"arena_block_size", "22"}, {"disable_auto_compactions", "true"}, - {"purge_redundant_kvs_while_flush", "1"}, {"compaction_style", "kCompactionStyleLevel"}, {"verify_checksums_in_compaction", "false"}, {"compaction_options_fifo", "23"}, @@ -216,7 +215,6 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) { ASSERT_EQ(new_cf_opt.hard_rate_limit, 2.1); ASSERT_EQ(new_cf_opt.arena_block_size, 22U); ASSERT_EQ(new_cf_opt.disable_auto_compactions, true); - ASSERT_EQ(new_cf_opt.purge_redundant_kvs_while_flush, true); ASSERT_EQ(new_cf_opt.compaction_style, kCompactionStyleLevel); ASSERT_EQ(new_cf_opt.verify_checksums_in_compaction, false); ASSERT_EQ(new_cf_opt.compaction_options_fifo.max_table_files_size,