From bec9ab4316002257263aef3db254e6ae926eb100 Mon Sep 17 00:00:00 2001 From: Baptiste Lemaire Date: Fri, 4 Feb 2022 05:31:07 -0800 Subject: [PATCH] Remove deprecated option DBOptions::max_mem_compaction_level (#9446) Summary: In RocksDB, this option was already marked as "NOT SUPPORTED" for a long time, and setting this option does not have any effect on the behavior of RocksDB library. Therefore, we are removing it in the preparations of the upcoming 7.0 release. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9446 Reviewed By: ajkr Differential Revision: D33793048 Pulled By: bjlemaire fbshipit-source-id: 73316efdb194e90225005246673dae99e65577ae --- HISTORY.md | 1 + db/c.cc | 3 --- include/rocksdb/advanced_options.h | 4 ---- include/rocksdb/c.h | 2 -- options/cf_options.cc | 6 ++++++ options/options_settable_test.cc | 17 ++++++++++++----- test_util/testutil.cc | 1 - 7 files changed, 19 insertions(+), 15 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 6ab23a3f1..708d7cbf2 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -24,6 +24,7 @@ * Removed timestamp from WriteOptions. Accordingly, added to DB APIs Put, Delete, SingleDelete, etc. accepting an additional argument 'timestamp'. Added Put, Delete, SingleDelete, etc to WriteBatch accepting an additional argument 'timestamp'. Removed WriteBatch::AssignTimestamps(vector) API. Renamed WriteBatch::AssignTimestamp() to WriteBatch::UpdateTimestamps() with clarified comments. * Remove default implementation of Name() from FileSystemWrapper. * Rename `SizeApproximationOptions.include_memtabtles` to `SizeApproximationOptions.include_memtables`. +* Remove deprecated option DBOptions::max_mem_compaction_level. ### Behavior Changes * Disallow the combination of DBOptions.use_direct_io_for_flush_and_compaction == true and DBOptions.writable_file_max_buffer_size == 0. This combination can cause WritableFileWriter::Append() to loop forever, and it does not make much sense in direct IO. diff --git a/db/c.cc b/db/c.cc index 4458d22a3..c99b9fc00 100644 --- a/db/c.cc +++ b/db/c.cc @@ -2805,9 +2805,6 @@ int rocksdb_options_get_level0_stop_writes_trigger(rocksdb_options_t* opt) { return opt->rep.level0_stop_writes_trigger; } -void rocksdb_options_set_max_mem_compaction_level(rocksdb_options_t* /*opt*/, - int /*n*/) {} - void rocksdb_options_set_wal_recovery_mode(rocksdb_options_t* opt,int mode) { opt->rep.wal_recovery_mode = static_cast(mode); } diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index 7cc146945..ccffb026d 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -917,10 +917,6 @@ struct AdvancedColumnFamilyOptions { explicit AdvancedColumnFamilyOptions(const Options& options); // ---------------- OPTIONS NOT SUPPORTED ANYMORE ---------------- - - // NOT SUPPORTED ANYMORE - // This does not do anything anymore. - int max_mem_compaction_level; }; } // namespace ROCKSDB_NAMESPACE diff --git a/include/rocksdb/c.h b/include/rocksdb/c.h index 7a74864a2..e26de3a50 100644 --- a/include/rocksdb/c.h +++ b/include/rocksdb/c.h @@ -1046,8 +1046,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_options_set_level0_stop_writes_trigger( rocksdb_options_t*, int); extern ROCKSDB_LIBRARY_API int rocksdb_options_get_level0_stop_writes_trigger( rocksdb_options_t*); -extern ROCKSDB_LIBRARY_API void rocksdb_options_set_max_mem_compaction_level( - rocksdb_options_t*, int); extern ROCKSDB_LIBRARY_API void rocksdb_options_set_target_file_size_base( rocksdb_options_t*, uint64_t); extern ROCKSDB_LIBRARY_API uint64_t diff --git a/options/cf_options.cc b/options/cf_options.cc index f7abb7874..0da7f449a 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -29,6 +29,11 @@ #include "rocksdb/utilities/options_type.h" #include "util/cast_util.h" +// NOTE: in this file, many option flags that were deprecated +// and removed from the rest of the code have to be kept here +// and marked as kDeprecated in order to be able to read old +// OPTIONS files. + namespace ROCKSDB_NAMESPACE { // offset_of is used to get the offset of a class data member // ex: offset_of(&ColumnFamilyOptions::num_levels) @@ -528,6 +533,7 @@ static std::unordered_map {offset_of(&ImmutableCFOptions::force_consistency_checks), OptionType::kBoolean, OptionVerificationType::kNormal, OptionTypeFlags::kNone}}, + // Need to keep this around to be able to read old OPTIONS files. {"max_mem_compaction_level", {0, OptionType::kInt, OptionVerificationType::kDeprecated, OptionTypeFlags::kNone}}, diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index 00899018e..d0585e5af 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -47,10 +47,16 @@ void FillWithSpecialChar(char* start_ptr, size_t total_size, const OffsetGap& excluded, char special_char = kSpecialChar) { size_t offset = 0; + // The excluded vector contains pairs of bytes, (first, second). + // The first bytes are all set to the special char (represented as 'c' below). + // The second bytes are simply skipped (padding bytes). + // ccccc[skipped]cccccccc[skiped]cccccccc[skipped] for (auto& pair : excluded) { std::memset(start_ptr + offset, special_char, pair.first - offset); offset = pair.first + pair.second; } + // The rest of the structure is filled with the special characters. + // ccccc[skipped]cccccccc[skiped]cccccccc[skipped]cccccccccccccccc std::memset(start_ptr + offset, special_char, total_size - offset); } @@ -59,6 +65,10 @@ int NumUnsetBytes(char* start_ptr, size_t total_size, int total_unset_bytes_base = 0; size_t offset = 0; for (auto& pair : excluded) { + // The first part of the structure contains memory spaces that can be + // set (pair.first), and memory spaces that cannot be set (pair.second). + // Therefore total_unset_bytes_base only agregates bytes set to kSpecialChar + // in the pair.first bytes, but skips the pair.second bytes (padding bytes). for (char* ptr = start_ptr + offset; ptr < start_ptr + pair.first; ptr++) { if (*ptr == kSpecialChar) { total_unset_bytes_base++; @@ -66,6 +76,8 @@ int NumUnsetBytes(char* start_ptr, size_t total_size, } offset = pair.first + pair.second; } + // Then total_unset_bytes_base aggregates the bytes + // set to kSpecialChar in the rest of the structure for (char* ptr = start_ptr + offset; ptr < start_ptr + total_size; ptr++) { if (*ptr == kSpecialChar) { total_unset_bytes_base++; @@ -422,10 +434,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { // which did in fact modify padding bytes. ColumnFamilyOptions* options = new (options_ptr) ColumnFamilyOptions(); - // Deprecatd option which is not initialized. Need to set it to avoid - // Valgrind error - options->max_mem_compaction_level = 0; - int unset_bytes_base = NumUnsetBytes(options_ptr, sizeof(ColumnFamilyOptions), kColumnFamilyOptionsExcluded); ASSERT_GT(unset_bytes_base, 0); @@ -439,7 +447,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { // GetColumnFamilyOptionsFromString(): options->compaction_options_universal = CompactionOptionsUniversal(); options->num_levels = 42; // Initialize options for MutableCF - options->max_mem_compaction_level = 0; options->compaction_filter = nullptr; options->sst_partitioner_factory = nullptr; diff --git a/test_util/testutil.cc b/test_util/testutil.cc index bd689d5bf..9596e10cd 100644 --- a/test_util/testutil.cc +++ b/test_util/testutil.cc @@ -408,7 +408,6 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, DBOptions& db_options, cf_opt->level0_slowdown_writes_trigger = rnd->Uniform(100); cf_opt->level0_stop_writes_trigger = rnd->Uniform(100); cf_opt->max_bytes_for_level_multiplier = rnd->Uniform(100); - cf_opt->max_mem_compaction_level = rnd->Uniform(100); cf_opt->max_write_buffer_number = rnd->Uniform(100); cf_opt->max_write_buffer_number_to_maintain = rnd->Uniform(100); cf_opt->max_write_buffer_size_to_maintain = rnd->Uniform(10000);