diff --git a/HISTORY.md b/HISTORY.md index 6cc0365641..b9ad1df581 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -7,7 +7,7 @@ ### New Features * Non-Blocking Manual Compaction (CompactRange()) - Support non-blocking manual compactions by setting a new CompactRangeOptions option (async_completion_cb). When set, the CompactRange() call will return control to the caller immediately. The manual compaction iteslf will be performed in an internally created thread. The manual compaction will ALWAYS call the specified callback upon completion and provide the completion status (#597). * Change the internal Configurable API SerializeOptions to return UserProperties (instead of the final string representation). Added ToString methods to the ConfigurableOptions class to complete the serialization of Options properties. - +* Added ConfigOptions::compare_to. When set, this value causes only values that have been changed to be part of the serialized output. ### Enhancements * Unit Testing: Expose the disallow_trivial_move flag in the MoveFilesToLevel testing utility (#677). diff --git a/include/rocksdb/convenience.h b/include/rocksdb/convenience.h index 0d662485ff..142d20cb10 100644 --- a/include/rocksdb/convenience.h +++ b/include/rocksdb/convenience.h @@ -107,6 +107,9 @@ struct ConfigOptions { // The object registry to use for this options std::shared_ptr registry; + // If set, only changes from this reference version will be serialized. + Configurable* compare_to = nullptr; + bool IsShallow() const { return depth == Depth::kDepthShallow; } bool IsDetailed() const { return depth == Depth::kDepthDetailed; } diff --git a/options/configurable.cc b/options/configurable.cc index 26fdc74baa..e9d6ccc809 100644 --- a/options/configurable.cc +++ b/options/configurable.cc @@ -541,16 +541,90 @@ Status ConfigurableHelper::SerializeOptions(const ConfigOptions& config_options, const std::string& prefix, OptionProperties* props) { assert(props); - for (auto const& opt_iter : configurable.options_) { - if (opt_iter.type_map != nullptr) { - Status s = OptionTypeInfo::SerializeType(config_options, prefix, - *(opt_iter.type_map), - opt_iter.opt_ptr, props); - if (!s.ok()) { - return s; + ConfigOptions copy = config_options; + auto compare_to = config_options.compare_to; + if (compare_to != nullptr && !MayBeEquivalent(configurable, *compare_to)) { + // If we are comparing this type to another, first see if the types + // are the same. If not, forget it + compare_to = nullptr; + } + + Status s; + for (size_t i = 0; i < configurable.options_.size(); i++) { + const auto& opt = configurable.options_[i]; + if (opt.type_map != nullptr) { + const auto opt_addr = opt.opt_ptr; + for (const auto& opt_iter : *(opt.type_map)) { + std::string single; + const auto& opt_name = opt_iter.first; + const auto& opt_info = opt_iter.second; + bool should_serialize = opt_info.ShouldSerialize(); + if (should_serialize && compare_to != nullptr) { + // This option should be serialized but there is a possiblity that it + // matches the default. Check to see if we really should serialize it + std::string mismatch; + if (opt_info.IsConfigurable() && + opt_info.IsEnabled(OptionTypeFlags::kStringNameOnly) && + !config_options.IsDetailed()) { + // If it is a Configurable name-only and we are not printing the + // details, then compare loosely + copy.sanity_level = ConfigOptions::kSanityLevelLooselyCompatible; + if (opt_info.AreEqual(copy, opt_name, opt_addr, + compare_to->options_[i].opt_ptr, &mismatch)) { + should_serialize = false; + } + copy.sanity_level = config_options.sanity_level; + } else if (opt_info.AreEqual(config_options, opt_name, opt_addr, + compare_to->options_[i].opt_ptr, + &mismatch)) { + should_serialize = false; + } + } + if (should_serialize) { + if (compare_to != nullptr && opt_info.IsCustomizable()) { + copy.compare_to = opt_info.AsRawPointer( + compare_to->options_[i].opt_ptr); + } else { + copy.compare_to = compare_to; + } + s = SerializeOption(copy, + OptionTypeInfo::MakePrefix(prefix, opt_name), + opt_info, opt_addr, &single); + if (!s.ok()) { + return s; + } else if (!single.empty()) { + props->insert_or_assign(opt_name, single); + } + } + } + } + } + return s; +} + +Status ConfigurableHelper::SerializeOption(const ConfigOptions& config_options, + const std::string& opt_name, + const OptionTypeInfo& opt_info, + const void* opt_addr, + std::string* value) { + if (opt_info.ShouldSerialize()) { + if (!config_options.mutable_options_only) { + return opt_info.Serialize(config_options, opt_name, opt_addr, value); + } else if (opt_info.IsMutable()) { + ConfigOptions copy = config_options; + copy.mutable_options_only = false; + return opt_info.Serialize(copy, opt_name, opt_addr, value); + } else if (opt_info.IsConfigurable()) { + // If it is a Configurable and we are either printing all of the + // details or not printing only the name, this option should be + // included in the list + if (config_options.IsDetailed() || + !opt_info.IsEnabled(OptionTypeFlags::kStringNameOnly)) { + return opt_info.Serialize(config_options, opt_name, opt_addr, value); } } } + value->clear(); return Status::OK(); } @@ -596,6 +670,25 @@ Status ConfigurableHelper::ListOptions( // //******************************************************************************* +bool ConfigurableHelper::MayBeEquivalent(const Configurable& this_one, + const Configurable& that_one) { + if (this_one.options_.size() != that_one.options_.size()) { + // The two types do not have the same number of registered options, + // therefore they cannot be the same. + return false; + } + + for (size_t i = 0; i < this_one.options_.size(); i++) { + const auto& this_opt = this_one.options_[i]; + const auto& that_opt = that_one.options_[i]; + if (this_opt.name != that_opt.name || + this_opt.type_map != that_opt.type_map) { + return false; + } + } + return true; +} + bool Configurable::AreEquivalent(const ConfigOptions& config_options, const Configurable* other, std::string* name) const { diff --git a/options/configurable_helper.h b/options/configurable_helper.h index cda7a3b0ea..cca7aa1884 100644 --- a/options/configurable_helper.h +++ b/options/configurable_helper.h @@ -152,6 +152,19 @@ class ConfigurableHelper { const std::string& prefix, OptionProperties* props); + // Serializes a single option to its string representation + // @param opt_name The name of the option + // @param opt_info The type and related information of the option + // @param opt_addr The address of the option + // @param value The string representation of the option. + // @return OK If the options for this object wer successfully serialized. + // @return InvalidArgument If one or more of the options could not be + // serialized. + static Status SerializeOption(const ConfigOptions& config_options, + const std::string& opt_name, + const OptionTypeInfo& opt_info, + const void* opt_addr, std::string* value); + // Internal method to list the option names for this object. // Classes may override this value to change its behavior. // @see ListOptions for more details @@ -173,6 +186,10 @@ class ConfigurableHelper { const Configurable& that_one, std::string* mismatch); + // Checks to see if the two Configurable classes may be equivalent + static bool MayBeEquivalent(const Configurable& this_one, + const Configurable& that_one); + private: // Looks for the option specified by name in the RegisteredOptions. // This method traverses the types in the input options vector. If an entry diff --git a/options/options_helper.cc b/options/options_helper.cc index a1507d6a50..edb5248c38 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -1144,30 +1144,14 @@ Status OptionTypeInfo::SerializeType( const ConfigOptions& config_options, const std::string& prefix, const std::unordered_map& type_map, const void* opt_addr, OptionProperties* props) { - Status status; for (const auto& iter : type_map) { std::string single; const auto& opt_name = iter.first; const auto& opt_info = iter.second; if (opt_info.ShouldSerialize()) { - if (!config_options.mutable_options_only) { - status = opt_info.Serialize( - config_options, MakePrefix(prefix, opt_name), opt_addr, &single); - } else if (opt_info.IsMutable()) { - ConfigOptions copy = config_options; - copy.mutable_options_only = false; - status = opt_info.Serialize(copy, MakePrefix(prefix, opt_name), - opt_addr, &single); - } else if (opt_info.IsConfigurable()) { - // If it is a Configurable and we are either printing all of the - // details or not printing only the name, this option should be - // included in the list - if (config_options.IsDetailed() || - !opt_info.IsEnabled(OptionTypeFlags::kStringNameOnly)) { - status = opt_info.Serialize( - config_options, MakePrefix(prefix, opt_name), opt_addr, &single); - } - } + Status status = ConfigurableHelper::SerializeOption( + config_options, MakePrefix(prefix, opt_name), opt_info, opt_addr, + &single); if (!status.ok()) { return status; } else if (!single.empty()) { @@ -1175,7 +1159,7 @@ Status OptionTypeInfo::SerializeType( } } } - return status; + return Status::OK(); } Status OptionTypeInfo::SerializeStruct( @@ -1305,7 +1289,8 @@ bool OptionTypeInfo::AreEqual(const ConfigOptions& config_options, const void* const that_ptr, std::string* mismatch) const { auto level = GetSanityLevel(); - if (!config_options.IsCheckEnabled(level)) { + if (config_options.compare_to == nullptr && + !config_options.IsCheckEnabled(level)) { return true; // If the sanity level is not being checked, skip it } if (this_ptr == nullptr || that_ptr == nullptr) { @@ -1332,7 +1317,8 @@ bool OptionTypeInfo::AreEqual(const ConfigOptions& config_options, } else if (this_config != nullptr && that_config != nullptr) { std::string bad_name; bool matches; - if (level < config_options.sanity_level) { + if (config_options.compare_to == nullptr && + level < config_options.sanity_level) { ConfigOptions copy = config_options; copy.sanity_level = level; matches = this_config->AreEquivalent(copy, that_config, &bad_name); @@ -1414,26 +1400,6 @@ bool OptionTypeInfo::StructsAreEqual( return matches; } -bool MatchesOptionsTypeFromMap( - const ConfigOptions& config_options, - const std::unordered_map& type_map, - const void* const this_ptr, const void* const that_ptr, - std::string* mismatch) { - for (auto& pair : type_map) { - // We skip checking deprecated variables as they might - // contain random values since they might not be initialized - if (config_options.IsCheckEnabled(pair.second.GetSanityLevel())) { - if (!pair.second.AreEqual(config_options, pair.first, this_ptr, that_ptr, - mismatch) && - !pair.second.AreEqualByName(config_options, pair.first, this_ptr, - that_ptr)) { - return false; - } - } - } - return true; -} - bool OptionTypeInfo::AreEqualByName(const ConfigOptions& config_options, const std::string& opt_name, const void* const this_ptr, diff --git a/options/options_test.cc b/options/options_test.cc index 8d1d67b4f0..f51e6f609e 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -47,6 +47,7 @@ #include "util/random.h" #include "util/stderr_logger.h" #include "util/string_util.h" +#include "utilities/merge_operators.h" #include "utilities/merge_operators/bytesxor.h" #include "utilities/merge_operators/sortlist.h" #include "utilities/merge_operators/string_append/stringappend.h" @@ -2270,6 +2271,215 @@ TEST_F(OptionsTest, OptionsListenerTest) { ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(config_opts, orig, copy)); } +static void TestDBOptionsChanged(const std::string& base_opts, + const std::string& changed_opts, + size_t changed) { + DBOptions base, copy; + ConfigOptions config; + std::string opts_str; + std::unordered_map opts_map; + + std::string trace_message; + if (!base_opts.empty()) { + trace_message = base_opts + "||" + changed_opts; + } else { + trace_message = changed_opts; + } + SCOPED_TRACE(trace_message.c_str()); + + if (!base_opts.empty()) { + ASSERT_OK(GetDBOptionsFromString(config, base, base_opts, &base)); + } + ASSERT_OK(GetDBOptionsFromString(config, base, changed_opts, ©)); + + auto dbcfg = DBOptionsAsConfigurable(base); + config.compare_to = dbcfg.get(); + config.sanity_level = ConfigOptions::kSanityLevelExactMatch; + config.depth = ConfigOptions::kDepthDetailed; + config.ignore_unknown_options = false; + config.ignore_unsupported_options = false; + + ASSERT_OK(GetStringFromDBOptions(config, copy, &opts_str)); + if (changed > 0) { + ASSERT_OK(StringToMap(opts_str, &opts_map)); + ASSERT_EQ(opts_map.size(), changed); + ASSERT_OK(GetDBOptionsFromMap(config, base, opts_map, &base)); + ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(config, base, copy)); + + dbcfg = DBOptionsAsConfigurable(base); + config.compare_to = dbcfg.get(); + ASSERT_OK(GetStringFromDBOptions(config, copy, &opts_str)); + } + ASSERT_EQ(opts_str, ""); +} + +TEST_F(OptionsTest, DBOptionsSerializeChangedOptions) { + TestDBOptionsChanged("", "", 0UL); // No changes + TestDBOptionsChanged("", "max_background_compactions=10", 1UL); + TestDBOptionsChanged( + "", "paranoid_checks=false; max_background_compactions=10", 2UL); + TestDBOptionsChanged( + "", "file_checksum_gen_factory=FileChecksumGenCrc32cFactory", 1UL); + TestDBOptionsChanged("file_checksum_gen_factory=FileChecksumGenCrc32cFactory", + "file_checksum_gen_factory=nullptr", 1UL); + TestDBOptionsChanged("file_checksum_gen_factory=FileChecksumGenCrc32cFactory", + "file_checksum_gen_factory=FileChecksumGenCrc32cFactory", + 0UL); +} + +static void TestCFOptionsChanged(const std::string& base_opts, + const std::string& changed_opts, + size_t changed) { + ColumnFamilyOptions base, copy; + ConfigOptions config; + std::string opts_str; + std::unordered_map opts_map; + + std::string trace_message; + if (!base_opts.empty()) { + trace_message = base_opts + "||" + changed_opts; + } else { + trace_message = changed_opts; + } + SCOPED_TRACE(trace_message.c_str()); + + if (!base_opts.empty()) { + ASSERT_OK(GetColumnFamilyOptionsFromString(config, base, base_opts, &base)); + } + ASSERT_OK( + GetColumnFamilyOptionsFromString(config, base, changed_opts, ©)); + + auto cfcfg = CFOptionsAsConfigurable(base); + config.compare_to = cfcfg.get(); + config.sanity_level = ConfigOptions::kSanityLevelExactMatch; + config.depth = ConfigOptions::kDepthDetailed; + config.ignore_unknown_options = false; + config.ignore_unsupported_options = false; + + ASSERT_OK(GetStringFromColumnFamilyOptions(config, copy, &opts_str)); + printf("MJR:[%s]=[%s]\n", trace_message.c_str(), opts_str.c_str()); + if (changed > 0) { + ASSERT_OK(StringToMap(opts_str, &opts_map)); + ASSERT_EQ(opts_map.size(), changed); + ASSERT_OK(GetColumnFamilyOptionsFromMap(config, base, opts_map, &base)); + ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config, base, copy)); + + cfcfg = CFOptionsAsConfigurable(base); + config.compare_to = cfcfg.get(); + ASSERT_OK(GetStringFromColumnFamilyOptions(config, copy, &opts_str)); + } + ASSERT_EQ(opts_str, ""); +} + +TEST_F(OptionsTest, CFOptionsSerializeChangedOptions) { + TestCFOptionsChanged("", "", 0UL); // No changes + TestCFOptionsChanged("", "compression=kXpressCompression", 1UL); + TestCFOptionsChanged("compression=kXpressCompression", + "compression=kSnappyCompression", 1UL); + TestCFOptionsChanged("", "comparator=rocksdb.ReverseBytewiseComparator", 1UL); + TestCFOptionsChanged("comparator=rocksdb.ReverseBytewiseComparator", + "comparator=leveldb.BytewiseComparator", 1UL); + TestCFOptionsChanged( + "", "merge_operator={id=StringAppendOperator;delimiter=|}", 1UL); + TestCFOptionsChanged("merge_operator={id=StringAppendOperator;delimiter=|}", + "merge_operator=nullptr", 1UL); + TestCFOptionsChanged("merge_operator={id=StringAppendOperator;delimiter=|}", + "merge_operator={id=StringAppendOperator;delimiter=%}", + 1UL); + TestCFOptionsChanged("", "block_based_table_factory={block_size=8192}", 1UL); + TestCFOptionsChanged("", "table_factory=PlainTable", 1UL); +} + +TEST_F(OptionsTest, SerializeChangedOptionsNameOnly) { + ColumnFamilyOptions base, copy; + ConfigOptions config; + std::string opts_str; + std::unordered_map opts_map; + + config.ignore_unknown_options = false; + config.ignore_unsupported_options = false; + config.depth = ConfigOptions::kDepthDefault; + + // Compare the table factories directly + copy.table_factory.reset(NewPlainTableFactory()); + opts_str = copy.table_factory->ToString(config); + + config.compare_to = base.table_factory.get(); + ASSERT_EQ(opts_str, copy.table_factory->ToString(config)); + + copy.table_factory.reset(NewBlockBasedTableFactory()); + auto bbto = copy.table_factory->GetOptions(); + ASSERT_NE(bbto, nullptr); + bbto->block_size = 123456; + auto tf_str = copy.table_factory->ToString(config); + ASSERT_OK(StringToMap(tf_str, &opts_map)); + ASSERT_EQ(opts_map.size(), 2); // block_size+id + opts_map.clear(); + + auto cfcfg = CFOptionsAsConfigurable(base); + config.compare_to = cfcfg.get(); + ASSERT_OK(GetStringFromColumnFamilyOptions(config, copy, &opts_str)); + ASSERT_EQ(opts_str, ""); + + // When not doing detailed, table factories do not compare sub-options + // so the options match + + config.depth = ConfigOptions::kDepthDetailed; + ASSERT_OK(GetStringFromColumnFamilyOptions(config, copy, &opts_str)); + ASSERT_OK(StringToMap(opts_str, &opts_map)); + ASSERT_EQ(opts_map.size(), 1); // table factory + ASSERT_EQ(tf_str, opts_map.begin()->second.c_str()); +} + +TEST_F(OptionsTest, SerializeChangedOptionsCompareLoosely) { + ColumnFamilyOptions base, copy; + ConfigOptions config; + std::string opts_str; + + config.ignore_unknown_options = false; + config.ignore_unsupported_options = false; + config.sanity_level = ConfigOptions::kSanityLevelLooselyCompatible; + + ASSERT_OK(MergeOperator::CreateFromString( + config, "id=StringAppendOperator;delimiter=|", &base.merge_operator)); + ASSERT_OK(MergeOperator::CreateFromString( + config, "{id=StringAppendOperator;delimiter=|}", ©.merge_operator)); + + auto copy_str = copy.merge_operator->ToString(config); + auto cfcfg = CFOptionsAsConfigurable(base); + + // Compare the merge operators directly + ASSERT_EQ(copy_str, base.merge_operator->ToString(config)); + config.compare_to = base.merge_operator.get(); + ASSERT_OK(copy.merge_operator->GetOptionString(config, &opts_str)); + ASSERT_EQ(opts_str, ""); + + config.compare_to = cfcfg.get(); + ASSERT_OK(GetStringFromColumnFamilyOptions(config, copy, &opts_str)); + ASSERT_EQ(opts_str, ""); + + ASSERT_OK(MergeOperator::CreateFromString( + config, "{id=StringAppendOperator;delimiter=%}", ©.merge_operator)); + copy_str = copy.merge_operator->ToString(config); + + config.compare_to = base.merge_operator.get(); + ASSERT_OK(copy.merge_operator->GetOptionString(config, &opts_str)); + ASSERT_EQ(opts_str, "delimiter=%"); + + ASSERT_OK(GetStringFromColumnFamilyOptions(config, base, &opts_str)); + + config.compare_to = cfcfg.get(); + ASSERT_OK(GetStringFromColumnFamilyOptions(config, base, &opts_str)); + ASSERT_EQ(opts_str, ""); + + std::unordered_map opts_map; + config.sanity_level = ConfigOptions::kSanityLevelExactMatch; + ASSERT_OK(GetStringFromColumnFamilyOptions(config, copy, &opts_str)); + ASSERT_OK(StringToMap(opts_str, &opts_map)); + ASSERT_EQ(opts_map.size(), 1UL); + ASSERT_EQ(copy_str, opts_map.begin()->second.c_str()); +} + const static std::string kCustomEnvName = "Custom"; const static std::string kCustomEnvProp = "env=" + kCustomEnvName;