From 78e82410eb8c22ccc626ee8dda7b2e96d08b0a1c Mon Sep 17 00:00:00 2001 From: mrambacher Date: Tue, 11 May 2021 16:14:33 -0700 Subject: [PATCH] Added static methods for simple types to OptionTypeInfo (#8249) Summary: Added ParseType, SerializeType, and TypesAreEqual methods to OptionTypeInfo. These methods can be used for serialization and deserialization of basic types. Change the MutableCF/DB Options to use this format. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8249 Reviewed By: jay-zhuang Differential Revision: D28351190 Pulled By: mrambacher fbshipit-source-id: 72a78643b804f2f0bf59c32ffefa63346672ad16 --- include/rocksdb/utilities/options_type.h | 48 +++++++ options/cf_options.cc | 25 ++++ options/cf_options.h | 11 ++ options/db_options.cc | 23 ++++ options/db_options.h | 11 ++ options/options_helper.cc | 162 ++++++++++++----------- options/options_helper.h | 17 --- options/options_test.cc | 67 ++++++++++ 8 files changed, 272 insertions(+), 92 deletions(-) diff --git a/include/rocksdb/utilities/options_type.h b/include/rocksdb/utilities/options_type.h index 9e93f394b..3ac0b553e 100644 --- a/include/rocksdb/utilities/options_type.h +++ b/include/rocksdb/utilities/options_type.h @@ -664,6 +664,37 @@ class OptionTypeInfo { const std::string& opt_name, const void* const this_ptr, const std::string& that_value) const; + // Parses the input opts_map according to the type_map for the opt_addr + // For each name-value pair in opts_map, find the corresponding name in + // type_map If the name is found: + // - set the corresponding value in opt_addr, returning the status on + // failure; + // If the name is not found: + // - If unused is specified, add the name-value to unused and continue + // - If ingore_unknown_options is false, return NotFound + // Returns OK if all options were either: + // - Successfully set + // - options were not found and ignore_unknown_options=true + // - options were not found and unused was specified + // Note that this method is much less sophisticated than the comparable + // Configurable::Configure methods. For example, on error, there is no + // attempt to return opt_addr to the initial state. Additionally, there + // is no effort to initialize (Configurable::PrepareOptions) the object + // on success. This method should typically only be used for simpler, + // standalone structures and not those that contain shared and embedded + // objects. + static Status ParseType( + const ConfigOptions& config_options, const std::string& opts_str, + const std::unordered_map& type_map, + void* opt_addr, + std::unordered_map* unused = nullptr); + static Status ParseType( + const ConfigOptions& config_options, + const std::unordered_map& opts_map, + const std::unordered_map& type_map, + void* opt_addr, + std::unordered_map* unused = nullptr); + // Parses the input value according to the map for the struct at opt_addr // struct_name is the name of the struct option as registered // opt_name is the name of the option being evaluated. This may @@ -674,6 +705,14 @@ class OptionTypeInfo { const std::unordered_map* map, const std::string& opt_name, const std::string& value, char* opt_addr); + // Serializes the values from opt_addr using the rules in type_map. + // Returns the serialized form in result. + // Returns OK on success or non-OK if some option could not be serialized. + static Status SerializeType( + const ConfigOptions& config_options, + const std::unordered_map& type_map, + const void* opt_addr, std::string* value); + // Serializes the input addr according to the map for the struct to value. // struct_name is the name of the struct option as registered // opt_name is the name of the option being evaluated. This may @@ -683,6 +722,15 @@ class OptionTypeInfo { const std::unordered_map* map, const std::string& opt_name, const char* opt_addr, std::string* value); + // Compares the values in this_addr and that_addr using the rules in type_map. + // If the values are equal, returns true + // If the values are not equal, returns false and sets mismatch to the name + // of the first value that did not match. + static bool TypesAreEqual( + const ConfigOptions& config_options, + const std::unordered_map& map, + const void* this_addr, const void* that_addr, std::string* mismatch); + // Compares the input offsets according to the map for the struct and returns // true if they are equivalent, false otherwise. // struct_name is the name of the struct option as registered diff --git a/options/cf_options.cc b/options/cf_options.cc index a77a18016..13e2d5520 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -1019,4 +1019,29 @@ void MutableCFOptions::Dump(Logger* log) const { MutableCFOptions::MutableCFOptions(const Options& options) : MutableCFOptions(ColumnFamilyOptions(options)) {} +#ifndef ROCKSDB_LITE +Status GetMutableOptionsFromStrings( + const MutableCFOptions& base_options, + const std::unordered_map& options_map, + Logger* /*info_log*/, MutableCFOptions* new_options) { + assert(new_options); + *new_options = base_options; + ConfigOptions config_options; + Status s = OptionTypeInfo::ParseType( + config_options, options_map, cf_mutable_options_type_info, new_options); + if (!s.ok()) { + *new_options = base_options; + } + return s; +} + +Status GetStringFromMutableCFOptions(const ConfigOptions& config_options, + const MutableCFOptions& mutable_opts, + std::string* opt_string) { + assert(opt_string); + opt_string->clear(); + return OptionTypeInfo::SerializeType( + config_options, cf_mutable_options_type_info, &mutable_opts, opt_string); +} +#endif // ROCKSDB_LITE } // namespace ROCKSDB_NAMESPACE diff --git a/options/cf_options.h b/options/cf_options.h index 0cb95e619..d4e77f04f 100644 --- a/options/cf_options.h +++ b/options/cf_options.h @@ -283,4 +283,15 @@ uint64_t MaxFileSizeForLevel(const MutableCFOptions& cf_options, // `pin_l0_filter_and_index_blocks_in_cache` is set. size_t MaxFileSizeForL0MetaPin(const MutableCFOptions& cf_options); +#ifndef ROCKSDB_LITE +Status GetStringFromMutableCFOptions(const ConfigOptions& config_options, + const MutableCFOptions& mutable_opts, + std::string* opt_string); + +Status GetMutableOptionsFromStrings( + const MutableCFOptions& base_options, + const std::unordered_map& options_map, + Logger* info_log, MutableCFOptions* new_options); +#endif // ROCKSDB_LITE + } // namespace ROCKSDB_NAMESPACE diff --git a/options/db_options.cc b/options/db_options.cc index d941e6c6c..c37e391d5 100644 --- a/options/db_options.cc +++ b/options/db_options.cc @@ -840,4 +840,27 @@ void MutableDBOptions::Dump(Logger* log) const { max_background_flushes); } +#ifndef ROCKSDB_LITE +Status GetMutableDBOptionsFromStrings( + const MutableDBOptions& base_options, + const std::unordered_map& options_map, + MutableDBOptions* new_options) { + assert(new_options); + *new_options = base_options; + ConfigOptions config_options; + Status s = OptionTypeInfo::ParseType( + config_options, options_map, db_mutable_options_type_info, new_options); + if (!s.ok()) { + *new_options = base_options; + } + return s; +} + +Status GetStringFromMutableDBOptions(const ConfigOptions& config_options, + const MutableDBOptions& mutable_opts, + std::string* opt_string) { + return OptionTypeInfo::SerializeType( + config_options, db_mutable_options_type_info, &mutable_opts, opt_string); +} +#endif // ROCKSDB_LITE } // namespace ROCKSDB_NAMESPACE diff --git a/options/db_options.h b/options/db_options.h index 5f18ddcb9..af1707a5a 100644 --- a/options/db_options.h +++ b/options/db_options.h @@ -129,4 +129,15 @@ struct MutableDBOptions { int max_background_flushes; }; +#ifndef ROCKSDB_LITE +Status GetStringFromMutableDBOptions(const ConfigOptions& config_options, + const MutableDBOptions& mutable_opts, + std::string* opt_string); + +Status GetMutableDBOptionsFromStrings( + const MutableDBOptions& base_options, + const std::unordered_map& options_map, + MutableDBOptions* new_options); +#endif // ROCKSDB_LITE + } // namespace ROCKSDB_NAMESPACE diff --git a/options/options_helper.cc b/options/options_helper.cc index 5ee8b4a7f..e6b4d7545 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -640,32 +640,6 @@ Status ConfigureFromMap( return s; } -Status GetMutableOptionsFromStrings( - const MutableCFOptions& base_options, - const std::unordered_map& options_map, - Logger* /*info_log*/, MutableCFOptions* new_options) { - assert(new_options); - *new_options = base_options; - ConfigOptions config_options; - const auto config = CFOptionsAsConfigurable(base_options); - return ConfigureFromMap(config_options, options_map, - MutableCFOptions::kName(), - config.get(), new_options); -} - -Status GetMutableDBOptionsFromStrings( - const MutableDBOptions& base_options, - const std::unordered_map& options_map, - MutableDBOptions* new_options) { - assert(new_options); - *new_options = base_options; - ConfigOptions config_options; - - auto config = DBOptionsAsConfigurable(base_options); - return ConfigureFromMap(config_options, options_map, - MutableDBOptions::kName(), - config.get(), new_options); -} Status StringToMap(const std::string& opts_str, std::unordered_map* opts_map) { @@ -707,12 +681,6 @@ Status StringToMap(const std::string& opts_str, return Status::OK(); } -Status GetStringFromMutableDBOptions(const ConfigOptions& config_options, - const MutableDBOptions& mutable_opts, - std::string* opt_string) { - auto config = DBOptionsAsConfigurable(mutable_opts); - return config->GetOptionString(config_options, opt_string); -} Status GetStringFromDBOptions(std::string* opt_string, const DBOptions& db_options, @@ -731,14 +699,6 @@ Status GetStringFromDBOptions(const ConfigOptions& config_options, return config->GetOptionString(config_options, opt_string); } -Status GetStringFromMutableCFOptions(const ConfigOptions& config_options, - const MutableCFOptions& mutable_opts, - std::string* opt_string) { - assert(opt_string); - opt_string->clear(); - const auto config = CFOptionsAsConfigurable(mutable_opts); - return config->GetOptionString(config_options, opt_string); -} Status GetStringFromColumnFamilyOptions(std::string* opt_string, const ColumnFamilyOptions& cf_options, @@ -1052,6 +1012,42 @@ Status OptionTypeInfo::Parse(const ConfigOptions& config_options, } } +Status OptionTypeInfo::ParseType( + const ConfigOptions& config_options, const std::string& opts_str, + const std::unordered_map& type_map, + void* opt_addr, std::unordered_map* unused) { + std::unordered_map opts_map; + Status status = StringToMap(opts_str, &opts_map); + if (!status.ok()) { + return status; + } else { + return ParseType(config_options, opts_map, type_map, opt_addr, unused); + } +} + +Status OptionTypeInfo::ParseType( + const ConfigOptions& config_options, + const std::unordered_map& opts_map, + const std::unordered_map& type_map, + void* opt_addr, std::unordered_map* unused) { + for (const auto& opts_iter : opts_map) { + std::string opt_name; + const auto* opt_info = Find(opts_iter.first, type_map, &opt_name); + if (opt_info != nullptr) { + Status status = + opt_info->Parse(config_options, opt_name, opts_iter.second, opt_addr); + if (!status.ok()) { + return status; + } + } else if (unused != nullptr) { + (*unused)[opts_iter.first] = opts_iter.second; + } else if (!config_options.ignore_unknown_options) { + return Status::NotFound("Unrecognized option", opts_iter.first); + } + } + return Status::OK(); +} + Status OptionTypeInfo::ParseStruct( const ConfigOptions& config_options, const std::string& struct_name, const std::unordered_map* struct_map, @@ -1060,20 +1056,12 @@ Status OptionTypeInfo::ParseStruct( Status status; if (opt_name == struct_name || EndsWith(opt_name, "." + struct_name)) { // This option represents the entire struct - std::unordered_map opt_map; - status = StringToMap(opt_value, &opt_map); - for (const auto& map_iter : opt_map) { - if (!status.ok()) { - break; - } - const auto iter = struct_map->find(map_iter.first); - if (iter != struct_map->end()) { - status = iter->second.Parse(config_options, map_iter.first, - map_iter.second, opt_addr); - } else { - status = Status::InvalidArgument("Unrecognized option", - struct_name + "." + map_iter.first); - } + std::unordered_map unused; + status = + ParseType(config_options, opt_value, *struct_map, opt_addr, &unused); + if (status.ok() && !unused.empty()) { + status = Status::InvalidArgument( + "Unrecognized option", struct_name + "." + unused.begin()->first); } } else if (StartsWith(opt_name, struct_name + ".")) { // This option represents a nested field in the struct (e.g, struct.field) @@ -1140,6 +1128,27 @@ Status OptionTypeInfo::Serialize(const ConfigOptions& config_options, } } +Status OptionTypeInfo::SerializeType( + const ConfigOptions& config_options, + const std::unordered_map& type_map, + const void* opt_addr, std::string* result) { + Status status; + for (const auto& iter : type_map) { + std::string single; + const auto& opt_info = iter.second; + if (opt_info.ShouldSerialize()) { + status = + opt_info.Serialize(config_options, iter.first, opt_addr, &single); + if (!status.ok()) { + return status; + } else { + result->append(iter.first + "=" + single + config_options.delimiter); + } + } + } + return status; +} + Status OptionTypeInfo::SerializeStruct( const ConfigOptions& config_options, const std::string& struct_name, const std::unordered_map* struct_map, @@ -1154,19 +1163,12 @@ Status OptionTypeInfo::SerializeStruct( // This option represents the entire struct std::string result; - for (const auto& iter : *struct_map) { - std::string single; - const auto& opt_info = iter.second; - if (opt_info.ShouldSerialize()) { - status = opt_info.Serialize(embedded, iter.first, opt_addr, &single); - if (!status.ok()) { - return status; - } else { - result.append(iter.first + "=" + single + embedded.delimiter); - } - } + status = SerializeType(embedded, *struct_map, opt_addr, &result); + if (!status.ok()) { + return status; + } else { + *value = "{" + result + "}"; } - *value = "{" + result + "}"; } else if (StartsWith(opt_name, struct_name + ".")) { // This option represents a nested field in the struct (e.g, struct.field) std::string elem_name; @@ -1304,6 +1306,20 @@ bool OptionTypeInfo::AreEqual(const ConfigOptions& config_options, return false; } +bool OptionTypeInfo::TypesAreEqual( + const ConfigOptions& config_options, + const std::unordered_map& type_map, + const void* this_addr, const void* that_addr, std::string* mismatch) { + for (const auto& iter : type_map) { + const auto& opt_info = iter.second; + if (!opt_info.AreEqual(config_options, iter.first, this_addr, that_addr, + mismatch)) { + return false; + } + } + return true; +} + bool OptionTypeInfo::StructsAreEqual( const ConfigOptions& config_options, const std::string& struct_name, const std::unordered_map* struct_map, @@ -1314,15 +1330,11 @@ bool OptionTypeInfo::StructsAreEqual( std::string result; if (EndsWith(opt_name, struct_name)) { // This option represents the entire struct - for (const auto& iter : *struct_map) { - const auto& opt_info = iter.second; - - matches = opt_info.AreEqual(config_options, iter.first, this_addr, - that_addr, &result); - if (!matches) { - *mismatch = struct_name + "." + result; - return false; - } + matches = TypesAreEqual(config_options, *struct_map, this_addr, that_addr, + &result); + if (!matches) { + *mismatch = struct_name + "." + result; + return false; } } else if (StartsWith(opt_name, struct_name + ".")) { // This option represents a nested field in the struct (e.g, struct.field) diff --git a/options/options_helper.h b/options/options_helper.h index fa3cab41d..a16c265ed 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -54,23 +54,6 @@ std::unique_ptr CFOptionsAsConfigurable( const ColumnFamilyOptions& opts, const std::unordered_map* opt_map = nullptr); -Status GetStringFromMutableCFOptions(const ConfigOptions& config_options, - const MutableCFOptions& mutable_opts, - std::string* opt_string); - -Status GetStringFromMutableDBOptions(const ConfigOptions& config_options, - const MutableDBOptions& mutable_opts, - std::string* opt_string); - -Status GetMutableOptionsFromStrings( - const MutableCFOptions& base_options, - const std::unordered_map& options_map, - Logger* info_log, MutableCFOptions* new_options); - -Status GetMutableDBOptionsFromStrings( - const MutableDBOptions& base_options, - const std::unordered_map& options_map, - MutableDBOptions* new_options); bool ParseSliceTransform( const std::string& value, diff --git a/options/options_test.cc b/options/options_test.cc index e87d887ac..d341af59e 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -1312,6 +1312,24 @@ TEST_F(OptionsTest, DBOptionsComposeImmutable) { new_opts)); } +TEST_F(OptionsTest, GetMutableDBOptions) { + Random rnd(228); + DBOptions base_opts; + std::string opts_str; + std::unordered_map opts_map; + ConfigOptions config_options; + + test::RandomInitDBOptions(&base_opts, &rnd); + ImmutableDBOptions i_opts(base_opts); + MutableDBOptions m_opts(base_opts); + MutableDBOptions new_opts; + ASSERT_OK(GetStringFromMutableDBOptions(config_options, m_opts, &opts_str)); + ASSERT_OK(StringToMap(opts_str, &opts_map)); + ASSERT_OK(GetMutableDBOptionsFromStrings(m_opts, opts_map, &new_opts)); + ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions( + config_options, base_opts, BuildDBOptions(i_opts, new_opts))); +} + TEST_F(OptionsTest, CFOptionsComposeImmutable) { // Build a DBOptions from an Immutable/Mutable one and verify that // we get same constituent options. @@ -1329,6 +1347,28 @@ TEST_F(OptionsTest, CFOptionsComposeImmutable) { delete new_opts.compaction_filter; } +TEST_F(OptionsTest, GetMutableCFOptions) { + Random rnd(228); + ColumnFamilyOptions base, copy; + std::string opts_str; + std::unordered_map opts_map; + ConfigOptions config_options; + DBOptions dummy; // Needed to create ImmutableCFOptions + + test::RandomInitCFOptions(&base, dummy, &rnd); + ColumnFamilyOptions result; + MutableCFOptions m_opts(base), new_opts; + + ASSERT_OK(GetStringFromMutableCFOptions(config_options, m_opts, &opts_str)); + ASSERT_OK(StringToMap(opts_str, &opts_map)); + ASSERT_OK(GetMutableOptionsFromStrings(m_opts, opts_map, nullptr, &new_opts)); + UpdateColumnFamilyOptions(ImmutableCFOptions(base), ©); + UpdateColumnFamilyOptions(new_opts, ©); + + ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config_options, base, copy)); + delete copy.compaction_filter; +} + TEST_F(OptionsTest, ColumnFamilyOptionsSerialization) { Options options; ColumnFamilyOptions base_opt, new_opt; @@ -3993,6 +4033,33 @@ TEST_F(OptionTypeInfoTest, TestVectorType) { ASSERT_EQ(vec1[2], "c1|c2|{d1|d2}"); } +TEST_F(OptionTypeInfoTest, TestStaticType) { + struct SimpleOptions { + size_t size = 0; + bool verify = true; + }; + + static std::unordered_map type_map = { + {"size", {offsetof(struct SimpleOptions, size), OptionType::kSizeT}}, + {"verify", + {offsetof(struct SimpleOptions, verify), OptionType::kBoolean}}, + }; + + ConfigOptions config_options; + SimpleOptions opts, copy; + opts.size = 12345; + opts.verify = false; + std::string str, mismatch; + + ASSERT_OK( + OptionTypeInfo::SerializeType(config_options, type_map, &opts, &str)); + ASSERT_FALSE(OptionTypeInfo::TypesAreEqual(config_options, type_map, &opts, + ©, &mismatch)); + ASSERT_OK(OptionTypeInfo::ParseType(config_options, str, type_map, ©)); + ASSERT_TRUE(OptionTypeInfo::TypesAreEqual(config_options, type_map, &opts, + ©, &mismatch)); +} + class ConfigOptionsTest : public testing::Test {}; TEST_F(ConfigOptionsTest, EnvFromConfigOptions) {