From 17f76fc5647ce1a06055c3eae8e5c8055749566f Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Wed, 14 Sep 2016 22:10:28 -0700 Subject: [PATCH] DB::GetOptions() reflect dynamic changed options Summary: DB::GetOptions() reflect dynamic changed options. Test Plan: See the new unit test. Reviewers: yhchiang, sdong, IslamAbdelRahman Reviewed By: IslamAbdelRahman Subscribers: andrewkr, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D63903 --- HISTORY.md | 4 + db/column_family.cc | 4 + db/column_family.h | 6 ++ db/db_impl.cc | 8 +- db/db_impl.h | 5 +- db/db_options_test.cc | 50 +++++++++ db/db_test.cc | 3 +- include/rocksdb/convenience.h | 2 +- include/rocksdb/db.h | 5 +- include/rocksdb/utilities/stackable_db.h | 3 +- util/options_helper.cc | 115 +++++++++++---------- util/options_helper.h | 13 ++- utilities/backupable/backupable_db_test.cc | 3 +- 13 files changed, 144 insertions(+), 77 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index cd7891195..6442bb883 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,4 +1,8 @@ # Rocksdb Change Log +## Unreleased +### Public API Change +* DB::GetOptions() reflect dynamic changed options (i.e. through DB::SetOptions()) and return copy of options instead of reference. + ## 4.12.0 (9/12/2016) ### Public API Change * CancelAllBackgroundWork() flushes all memtables for databases containing writes that have bypassed the WAL (writes issued with WriteOptions::disableWAL=true) before shutting down background threads. diff --git a/db/column_family.cc b/db/column_family.cc index 54f59d8cb..673e06a8d 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -482,6 +482,10 @@ void ColumnFamilyData::SetDropped() { column_family_set_->RemoveColumnFamily(this); } +ColumnFamilyOptions ColumnFamilyData::GetLatestCFOptions() const { + return BuildColumnFamilyOptions(options_, mutable_cf_options_); +} + const double kSlowdownRatio = 1.2; namespace { diff --git a/db/column_family.h b/db/column_family.h index 66d0448e1..1f3642c11 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -218,6 +218,12 @@ class ColumnFamilyData { const MutableCFOptions* GetLatestMutableCFOptions() const { return &mutable_cf_options_; } + + // REQUIRES: DB mutex held + // Build ColumnFamiliesOptions with immutable options and latest mutable + // options. + ColumnFamilyOptions GetLatestCFOptions() const; + #ifndef ROCKSDB_LITE // REQUIRES: DB mutex held Status SetOptions( diff --git a/db/db_impl.cc b/db/db_impl.cc index 2b2cbfad8..ccc8391ec 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -5110,9 +5110,10 @@ Env* DBImpl::GetEnv() const { return env_; } -const Options& DBImpl::GetOptions(ColumnFamilyHandle* column_family) const { +Options DBImpl::GetOptions(ColumnFamilyHandle* column_family) const { + InstrumentedMutexLock l(&mutex_); auto cfh = reinterpret_cast(column_family); - return *cfh->cfd()->options(); + return Options(db_options_, cfh->cfd()->GetLatestCFOptions()); } const DBOptions& DBImpl::GetDBOptions() const { return db_options_; } @@ -5993,8 +5994,7 @@ Status DBImpl::WriteOptionsFile() { continue; } cf_names.push_back(cfd->GetName()); - cf_opts.push_back(BuildColumnFamilyOptions( - *cfd->options(), *cfd->GetLatestMutableCFOptions())); + cf_opts.push_back(cfd->GetLatestCFOptions()); } // Unlock during expensive operations. New writes cannot get here diff --git a/db/db_impl.h b/db/db_impl.h index 7986c1da0..51b058d51 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -165,8 +165,7 @@ class DBImpl : public DB { virtual const std::string& GetName() const override; virtual Env* GetEnv() const override; using DB::GetOptions; - virtual const Options& GetOptions( - ColumnFamilyHandle* column_family) const override; + virtual Options GetOptions(ColumnFamilyHandle* column_family) const override; using DB::GetDBOptions; virtual const DBOptions& GetDBOptions() const override; using DB::Flush; @@ -717,7 +716,7 @@ class DBImpl : public DB { // same time. InstrumentedMutex options_files_mutex_; // State below is protected by mutex_ - InstrumentedMutex mutex_; + mutable InstrumentedMutex mutex_; std::atomic shutting_down_; // This condition variable is signaled on these conditions: diff --git a/db/db_options_test.cc b/db/db_options_test.cc index da65a5b67..b5b017a56 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -8,21 +8,71 @@ // found in the LICENSE file. See the AUTHORS file for names of contributors. #include #include +#include +#include "db/column_family.h" #include "db/db_test_util.h" #include "port/stack_trace.h" +#include "rocksdb/convenience.h" +#include "util/options_helper.h" +#include "util/random.h" #include "util/sync_point.h" +#include "util/testutil.h" namespace rocksdb { class DBOptionsTest : public DBTestBase { public: DBOptionsTest() : DBTestBase("/db_options_test") {} + +#ifndef ROCKSDB_LITE + std::unordered_map GetMutableCFOptionsMap( + const ColumnFamilyOptions& options) { + std::string options_str; + GetStringFromColumnFamilyOptions(&options_str, options); + std::unordered_map options_map; + StringToMap(options_str, &options_map); + std::unordered_map mutable_map; + for (const auto opt : cf_options_type_info) { + if (opt.second.is_mutable && + opt.second.verification != OptionVerificationType::kDeprecated) { + mutable_map[opt.first] = options_map[opt.first]; + } + } + return mutable_map; + } + + std::unordered_map GetRandomizedMutableCFOptionsMap( + Random* rnd) { + Options options; + test::RandomInitCFOptions(&options, rnd); + auto sanitized_options = SanitizeOptions(options, nullptr, options); + return GetMutableCFOptionsMap(sanitized_options); + } +#endif // ROCKSDB_LITE }; // RocksDB lite don't support dynamic options. #ifndef ROCKSDB_LITE +TEST_F(DBOptionsTest, GetLatestOptions) { + // GetOptions should be able to get latest option changed by SetOptions. + Options options; + options.create_if_missing = true; + Random rnd(228); + Reopen(options); + CreateColumnFamilies({"foo"}, options); + ReopenWithColumnFamilies({"default", "foo"}, options); + auto options_default = GetRandomizedMutableCFOptionsMap(&rnd); + auto options_foo = GetRandomizedMutableCFOptionsMap(&rnd); + ASSERT_OK(dbfull()->SetOptions(handles_[0], options_default)); + ASSERT_OK(dbfull()->SetOptions(handles_[1], options_foo)); + ASSERT_EQ(options_default, + GetMutableCFOptionsMap(dbfull()->GetOptions(handles_[0]))); + ASSERT_EQ(options_foo, + GetMutableCFOptionsMap(dbfull()->GetOptions(handles_[1]))); +} + TEST_F(DBOptionsTest, EnableAutoCompactionAndTriggerStall) { const std::string kValue(1024, 'v'); for (int method_type = 0; method_type < 2; method_type++) { diff --git a/db/db_test.cc b/db/db_test.cc index f766c0804..32e12bad9 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2806,8 +2806,7 @@ class ModelDB : public DB { virtual Env* GetEnv() const override { return nullptr; } using DB::GetOptions; - virtual const Options& GetOptions( - ColumnFamilyHandle* column_family) const override { + virtual Options GetOptions(ColumnFamilyHandle* column_family) const override { return options_; } diff --git a/include/rocksdb/convenience.h b/include/rocksdb/convenience.h index fa785b4c5..2cbaa375b 100644 --- a/include/rocksdb/convenience.h +++ b/include/rocksdb/convenience.h @@ -282,7 +282,7 @@ Status GetStringFromDBOptions(std::string* opts_str, const std::string& delimiter = "; "); Status GetStringFromColumnFamilyOptions(std::string* opts_str, - const ColumnFamilyOptions& db_options, + const ColumnFamilyOptions& cf_options, const std::string& delimiter = "; "); Status GetStringFromCompressionType(std::string* compression_str, diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index 280e919e4..534c120cd 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -696,9 +696,8 @@ class DB { // column family, the options provided when calling DB::Open() or // DB::CreateColumnFamily() will have been "sanitized" and transformed // in an implementation-defined manner. - virtual const Options& GetOptions(ColumnFamilyHandle* column_family) - const = 0; - virtual const Options& GetOptions() const { + virtual Options GetOptions(ColumnFamilyHandle* column_family) const = 0; + virtual Options GetOptions() const { return GetOptions(DefaultColumnFamily()); } diff --git a/include/rocksdb/utilities/stackable_db.h b/include/rocksdb/utilities/stackable_db.h index 455246b53..68a729d23 100644 --- a/include/rocksdb/utilities/stackable_db.h +++ b/include/rocksdb/utilities/stackable_db.h @@ -219,8 +219,7 @@ class StackableDB : public DB { } using DB::GetOptions; - virtual const Options& GetOptions(ColumnFamilyHandle* column_family) const - override { + virtual Options GetOptions(ColumnFamilyHandle* column_family) const override { return db_->GetOptions(column_family); } diff --git a/util/options_helper.cc b/util/options_helper.cc index 20b2ea484..8ec3e1fbb 100644 --- a/util/options_helper.cc +++ b/util/options_helper.cc @@ -26,6 +26,65 @@ namespace rocksdb { +ColumnFamilyOptions BuildColumnFamilyOptions( + const ColumnFamilyOptions& options, + const MutableCFOptions& mutable_cf_options) { + ColumnFamilyOptions cf_opts(options); + + // Memtable related options + cf_opts.write_buffer_size = mutable_cf_options.write_buffer_size; + cf_opts.max_write_buffer_number = mutable_cf_options.max_write_buffer_number; + cf_opts.arena_block_size = mutable_cf_options.arena_block_size; + cf_opts.memtable_prefix_bloom_size_ratio = + mutable_cf_options.memtable_prefix_bloom_size_ratio; + cf_opts.memtable_huge_page_size = mutable_cf_options.memtable_huge_page_size; + cf_opts.max_successive_merges = mutable_cf_options.max_successive_merges; + cf_opts.inplace_update_num_locks = + mutable_cf_options.inplace_update_num_locks; + + // Compaction related options + cf_opts.disable_auto_compactions = + mutable_cf_options.disable_auto_compactions; + cf_opts.level0_file_num_compaction_trigger = + mutable_cf_options.level0_file_num_compaction_trigger; + cf_opts.level0_slowdown_writes_trigger = + mutable_cf_options.level0_slowdown_writes_trigger; + cf_opts.level0_stop_writes_trigger = + mutable_cf_options.level0_stop_writes_trigger; + cf_opts.max_compaction_bytes = mutable_cf_options.max_compaction_bytes; + cf_opts.target_file_size_base = mutable_cf_options.target_file_size_base; + cf_opts.target_file_size_multiplier = + mutable_cf_options.target_file_size_multiplier; + cf_opts.max_bytes_for_level_base = + mutable_cf_options.max_bytes_for_level_base; + cf_opts.max_bytes_for_level_multiplier = + mutable_cf_options.max_bytes_for_level_multiplier; + + cf_opts.max_bytes_for_level_multiplier_additional.clear(); + for (auto value : + mutable_cf_options.max_bytes_for_level_multiplier_additional) { + cf_opts.max_bytes_for_level_multiplier_additional.emplace_back(value); + } + + cf_opts.verify_checksums_in_compaction = + mutable_cf_options.verify_checksums_in_compaction; + + // Misc options + cf_opts.max_sequential_skip_in_iterations = + mutable_cf_options.max_sequential_skip_in_iterations; + cf_opts.paranoid_file_checks = mutable_cf_options.paranoid_file_checks; + cf_opts.report_bg_io_stats = mutable_cf_options.report_bg_io_stats; + cf_opts.compression = mutable_cf_options.compression; + cf_opts.min_partial_merge_operands = + mutable_cf_options.min_partial_merge_operands; + + cf_opts.table_factory = options.table_factory; + // TODO(yhchiang): find some way to handle the following derived options + // * max_file_size + + return cf_opts; +} + #ifndef ROCKSDB_LITE bool isSpecialChar(const char c) { if (c == '\\' || c == '#' || c == ':' || c == '\r' || c == '\n') { @@ -1367,60 +1426,6 @@ Status GetTableFactoryFromMap( return Status::OK(); } -ColumnFamilyOptions BuildColumnFamilyOptions( - const Options& options, const MutableCFOptions& mutable_cf_options) { - ColumnFamilyOptions cf_opts(options); - - // Memtable related options - cf_opts.write_buffer_size = mutable_cf_options.write_buffer_size; - cf_opts.max_write_buffer_number = mutable_cf_options.max_write_buffer_number; - cf_opts.arena_block_size = mutable_cf_options.arena_block_size; - cf_opts.memtable_prefix_bloom_size_ratio = - mutable_cf_options.memtable_prefix_bloom_size_ratio; - cf_opts.memtable_huge_page_size = mutable_cf_options.memtable_huge_page_size; - cf_opts.max_successive_merges = mutable_cf_options.max_successive_merges; - cf_opts.inplace_update_num_locks = - mutable_cf_options.inplace_update_num_locks; - - // Compaction related options - cf_opts.disable_auto_compactions = - mutable_cf_options.disable_auto_compactions; - cf_opts.level0_file_num_compaction_trigger = - mutable_cf_options.level0_file_num_compaction_trigger; - cf_opts.level0_slowdown_writes_trigger = - mutable_cf_options.level0_slowdown_writes_trigger; - cf_opts.level0_stop_writes_trigger = - mutable_cf_options.level0_stop_writes_trigger; - cf_opts.max_compaction_bytes = mutable_cf_options.max_compaction_bytes; - cf_opts.target_file_size_base = mutable_cf_options.target_file_size_base; - cf_opts.target_file_size_multiplier = - mutable_cf_options.target_file_size_multiplier; - cf_opts.max_bytes_for_level_base = - mutable_cf_options.max_bytes_for_level_base; - cf_opts.max_bytes_for_level_multiplier = - mutable_cf_options.max_bytes_for_level_multiplier; - - cf_opts.max_bytes_for_level_multiplier_additional.clear(); - for (auto value : - mutable_cf_options.max_bytes_for_level_multiplier_additional) { - cf_opts.max_bytes_for_level_multiplier_additional.emplace_back(value); - } - - cf_opts.verify_checksums_in_compaction = - mutable_cf_options.verify_checksums_in_compaction; - - // Misc options - cf_opts.max_sequential_skip_in_iterations = - mutable_cf_options.max_sequential_skip_in_iterations; - cf_opts.paranoid_file_checks = mutable_cf_options.paranoid_file_checks; - cf_opts.report_bg_io_stats = mutable_cf_options.report_bg_io_stats; - - cf_opts.table_factory = options.table_factory; - // TODO(yhchiang): find some way to handle the following derived options - // * max_file_size - - return cf_opts; -} - #endif // !ROCKSDB_LITE + } // namespace rocksdb diff --git a/util/options_helper.h b/util/options_helper.h index 1e8ac6088..2f7cde7e6 100644 --- a/util/options_helper.h +++ b/util/options_helper.h @@ -14,9 +14,14 @@ #include "rocksdb/table.h" #include "util/cf_options.h" -#ifndef ROCKSDB_LITE namespace rocksdb { +ColumnFamilyOptions BuildColumnFamilyOptions( + const ColumnFamilyOptions& ioptions, + const MutableCFOptions& mutable_cf_options); + +#ifndef ROCKSDB_LITE + // Returns true if the input char "c" is considered as a special character // that will be escaped when EscapeOptionString() is called. // @@ -68,9 +73,6 @@ Status GetTableFactoryFromMap( Status GetStringFromTableFactory(std::string* opts_str, const TableFactory* tf, const std::string& delimiter = "; "); -ColumnFamilyOptions BuildColumnFamilyOptions( - const Options& options, const MutableCFOptions& mutable_cf_options); - enum class OptionType { kBoolean, kInt, @@ -672,6 +674,7 @@ static std::unordered_map info_log_level_string_map = {"FATAL_LEVEL", InfoLogLevel::FATAL_LEVEL}, {"HEADER_LEVEL", InfoLogLevel::HEADER_LEVEL}}; +#endif // !ROCKSDB_LITE + } // namespace rocksdb -#endif // !ROCKSDB_LITE diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backupable/backupable_db_test.cc index 6af0c7057..86f5dca24 100644 --- a/utilities/backupable/backupable_db_test.cc +++ b/utilities/backupable/backupable_db_test.cc @@ -57,8 +57,7 @@ class DummyDB : public StackableDB { } using DB::GetOptions; - virtual const Options& GetOptions(ColumnFamilyHandle* column_family) const - override { + virtual Options GetOptions(ColumnFamilyHandle* column_family) const override { return options_; }