From 6650ca244ed27e852e8325f593faf863eaeb47da Mon Sep 17 00:00:00 2001 From: Hui Xiao Date: Tue, 7 Feb 2023 14:11:53 -0800 Subject: [PATCH] Remove a couple deprecated convenience.h APIs (#11120) Summary: **Context/Summary:** As instructed by convenience.h comments, a few deprecated APIs are removed. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11120 Test Plan: - make check & CI - eyeball check on test semantics. Reviewed By: pdillinger Differential Revision: D42937507 Pulled By: hx235 fbshipit-source-id: a9e4709387da01b1d0e9148c2e210f02e9746ee1 --- HISTORY.md | 1 + include/rocksdb/convenience.h | 91 +--- java/rocksjni/options.cc | 12 +- options/options_helper.cc | 46 -- options/options_settable_test.cc | 17 +- options/options_test.cc | 439 ++++++++++-------- .../block_based/block_based_table_factory.cc | 26 -- table/plain/plain_table_factory.cc | 23 - 8 files changed, 301 insertions(+), 354 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 0b4634885..0637747a0 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -23,6 +23,7 @@ ### Public API Changes * Completely removed the following deprecated/obsolete statistics: the tickers `BLOCK_CACHE_INDEX_BYTES_EVICT`, `BLOCK_CACHE_FILTER_BYTES_EVICT`, `BLOOM_FILTER_MICROS`, `NO_FILE_CLOSES`, `STALL_L0_SLOWDOWN_MICROS`, `STALL_MEMTABLE_COMPACTION_MICROS`, `STALL_L0_NUM_FILES_MICROS`, `RATE_LIMIT_DELAY_MILLIS`, `NO_ITERATORS`, `NUMBER_FILTERED_DELETES`, `WRITE_TIMEDOUT`, `BLOB_DB_GC_NUM_KEYS_OVERWRITTEN`, `BLOB_DB_GC_NUM_KEYS_EXPIRED`, `BLOB_DB_GC_BYTES_OVERWRITTEN`, `BLOB_DB_GC_BYTES_EXPIRED`, `BLOCK_CACHE_COMPRESSION_DICT_BYTES_EVICT` as well as the histograms `STALL_L0_SLOWDOWN_COUNT`, `STALL_MEMTABLE_COMPACTION_COUNT`, `STALL_L0_NUM_FILES_COUNT`, `HARD_RATE_LIMIT_DELAY_COUNT`, `SOFT_RATE_LIMIT_DELAY_COUNT`, `BLOB_DB_GC_MICROS`, and `NUM_DATA_BLOCKS_READ_PER_LEVEL`. Note that as a result, the C++ enum values of the still supported statistics have changed. Developers are advised to not rely on the actual numeric values. * Deprecated IngestExternalFileOptions::write_global_seqno and change default to false. This option only needs to be set to true to generate a DB compatible with RocksDB versions before 5.16.0. +* Remove deprecated APIs `GetColumnFamilyOptionsFrom{Map|String}(const ColumnFamilyOptions&, ..)`, `GetDBOptionsFrom{Map|String}(const DBOptions&, ..)`, `GetBlockBasedTableOptionsFrom{Map|String}(const BlockBasedTableOptions& table_options, ..)` and ` GetPlainTableOptionsFrom{Map|String}(const PlainTableOptions& table_options,..)`. ### Build Changes * The `make` build now builds a shared library by default instead of a static library. Use `LIB_MODE=static` to override. diff --git a/include/rocksdb/convenience.h b/include/rocksdb/convenience.h index fb740c6fc..7ce676df0 100644 --- a/include/rocksdb/convenience.h +++ b/include/rocksdb/convenience.h @@ -160,9 +160,10 @@ struct ConfigOptions { // "kCompactionStyleNone". // -// Take a default ColumnFamilyOptions "base_options" in addition to a -// map "opts_map" of option name to option value to construct the new -// ColumnFamilyOptions "new_options". +// Take a ConfigOptions `config_options` and a ColumnFamilyOptions +// "base_options" as the default option in addition to a map "opts_map" of +// option name to option value to construct the new ColumnFamilyOptions +// "new_options". // // Below are the instructions of how to config some non-primitive-typed // options in ColumnFamilyOptions: @@ -238,11 +239,6 @@ struct ConfigOptions { // cf_opt.compression_opts.strategy = 6; // cf_opt.compression_opts.max_dict_bytes = 7; // -// The GetColumnFamilyOptionsFromMap(ConfigOptions, ...) should be used; the -// alternative signature may be deprecated in a future release. The equivalent -// functionality can be achieved by setting the corresponding options in -// the ConfigOptions parameter. -// // @param config_options controls how the map is processed. // @param base_options the default options of the output "new_options". // @param opts_map an option name to value map for specifying how "new_options" @@ -267,15 +263,10 @@ Status GetColumnFamilyOptionsFromMap( const ColumnFamilyOptions& base_options, const std::unordered_map& opts_map, ColumnFamilyOptions* new_options); -Status GetColumnFamilyOptionsFromMap( - const ColumnFamilyOptions& base_options, - const std::unordered_map& opts_map, - ColumnFamilyOptions* new_options, bool input_strings_escaped = false, - bool ignore_unknown_options = false); -// Take a default DBOptions "base_options" in addition to a -// map "opts_map" of option name to option value to construct the new -// DBOptions "new_options". +// Take a ConfigOptions `config_options` and a DBOptions "base_options" as the +// default option in addition to a map "opts_map" of option name to option value +// to construct the new DBOptions "new_options". // // Below are the instructions of how to config some non-primitive-typed // options in DBOptions: @@ -286,11 +277,6 @@ Status GetColumnFamilyOptionsFromMap( // - Passing {"rate_limiter_bytes_per_sec", "1024"} is equivalent to // passing NewGenericRateLimiter(1024) to rate_limiter_bytes_per_sec. // -// The GetDBOptionsFromMap(ConfigOptions, ...) should be used; the -// alternative signature may be deprecated in a future release. The equivalent -// functionality can be achieved by setting the corresponding options in -// the ConfigOptions parameter. -// // @param config_options controls how the map is processed. // @param base_options the default options of the output "new_options". // @param opts_map an option name to value map for specifying how "new_options" @@ -314,15 +300,11 @@ Status GetDBOptionsFromMap( const ConfigOptions& cfg_options, const DBOptions& base_options, const std::unordered_map& opts_map, DBOptions* new_options); -Status GetDBOptionsFromMap( - const DBOptions& base_options, - const std::unordered_map& opts_map, - DBOptions* new_options, bool input_strings_escaped = false, - bool ignore_unknown_options = false); -// Take a default BlockBasedTableOptions "table_options" in addition to a -// map "opts_map" of option name to option value to construct the new -// BlockBasedTableOptions "new_table_options". +// Take a ConfigOptions `config_options` and a BlockBasedTableOptions +// "table_options" as the default option in addition to a map "opts_map" of +// option name to option value to construct the new BlockBasedTableOptions +// "new_table_options". // // Below are the instructions of how to config some non-primitive-typed // options in BlockBasedTableOptions: @@ -345,11 +327,6 @@ Status GetDBOptionsFromMap( // - Passing {"block_cache", "1M"} in GetBlockBasedTableOptionsFromMap is // equivalent to setting block_cache using NewLRUCache(1024 * 1024). // -// The GetBlockBasedTableOptionsFromMap(ConfigOptions, ...) should be used; -// the alternative signature may be deprecated in a future release. The -// equivalent functionality can be achieved by setting the corresponding -// options in the ConfigOptions parameter. -// // @param config_options controls how the map is processed. // @param table_options the default options of the output "new_table_options". // @param opts_map an option name to value map for specifying how @@ -369,20 +346,11 @@ Status GetBlockBasedTableOptionsFromMap( const BlockBasedTableOptions& table_options, const std::unordered_map& opts_map, BlockBasedTableOptions* new_table_options); -Status GetBlockBasedTableOptionsFromMap( - const BlockBasedTableOptions& table_options, - const std::unordered_map& opts_map, - BlockBasedTableOptions* new_table_options, - bool input_strings_escaped = false, bool ignore_unknown_options = false); -// Take a default PlainTableOptions "table_options" in addition to a -// map "opts_map" of option name to option value to construct the new -// PlainTableOptions "new_table_options". -// -// The GetPlainTableOptionsFromMap(ConfigOptions, ...) should be used; the -// alternative signature may be deprecated in a future release. The equivalent -// functionality can be achieved by setting the corresponding options in -// the ConfigOptions parameter. +// Take a ConfigOptions `config_options` and a default PlainTableOptions +// "table_options" as the default option in addition to a map "opts_map" of +// option name to option value to construct the new PlainTableOptions +// "new_table_options". // // @param config_options controls how the map is processed. // @param table_options the default options of the output "new_table_options". @@ -402,43 +370,26 @@ Status GetPlainTableOptionsFromMap( const ConfigOptions& config_options, const PlainTableOptions& table_options, const std::unordered_map& opts_map, PlainTableOptions* new_table_options); -Status GetPlainTableOptionsFromMap( - const PlainTableOptions& table_options, - const std::unordered_map& opts_map, - PlainTableOptions* new_table_options, bool input_strings_escaped = false, - bool ignore_unknown_options = false); -// Take a string representation of option names and values, apply them into the -// base_options, and return the new options as a result. The string has the -// following format: +// Take a ConfigOptions `config_options`, a string representation of option +// names and values, apply them into the base_options, and return the new +// options as a result. The string has the following format: // "write_buffer_size=1024;max_write_buffer_number=2" // Nested options config is also possible. For example, you can define // BlockBasedTableOptions as part of the string for block-based table factory: // "write_buffer_size=1024;block_based_table_factory={block_size=4k};" // "max_write_buffer_num=2" // -// -// The GetColumnFamilyOptionsFromString(ConfigOptions, ...) should be used; the -// alternative signature may be deprecated in a future release. The equivalent -// functionality can be achieved by setting the corresponding options in -// the ConfigOptions parameter. Status GetColumnFamilyOptionsFromString(const ConfigOptions& config_options, const ColumnFamilyOptions& base_options, const std::string& opts_str, ColumnFamilyOptions* new_options); -Status GetColumnFamilyOptionsFromString(const ColumnFamilyOptions& base_options, - const std::string& opts_str, - ColumnFamilyOptions* new_options); Status GetDBOptionsFromString(const ConfigOptions& config_options, const DBOptions& base_options, const std::string& opts_str, DBOptions* new_options); -Status GetDBOptionsFromString(const DBOptions& base_options, - const std::string& opts_str, - DBOptions* new_options); - Status GetStringFromDBOptions(const ConfigOptions& config_options, const DBOptions& db_options, std::string* opts_str); @@ -458,17 +409,11 @@ Status GetStringFromCompressionType(std::string* compression_str, std::vector GetSupportedCompressions(); -Status GetBlockBasedTableOptionsFromString( - const BlockBasedTableOptions& table_options, const std::string& opts_str, - BlockBasedTableOptions* new_table_options); Status GetBlockBasedTableOptionsFromString( const ConfigOptions& config_options, const BlockBasedTableOptions& table_options, const std::string& opts_str, BlockBasedTableOptions* new_table_options); -Status GetPlainTableOptionsFromString(const PlainTableOptions& table_options, - const std::string& opts_str, - PlainTableOptions* new_table_options); Status GetPlainTableOptionsFromString(const ConfigOptions& config_options, const PlainTableOptions& table_options, const std::string& opts_str, diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index b848ea9cf..724d298e7 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -3990,9 +3990,13 @@ jlong Java_org_rocksdb_ColumnFamilyOptions_getColumnFamilyOptionsFromProps__Ljav } auto* cf_options = new ROCKSDB_NAMESPACE::ColumnFamilyOptions(); + ROCKSDB_NAMESPACE::ConfigOptions config_options; + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; ROCKSDB_NAMESPACE::Status status = ROCKSDB_NAMESPACE::GetColumnFamilyOptionsFromString( - ROCKSDB_NAMESPACE::ColumnFamilyOptions(), opt_string, cf_options); + config_options, ROCKSDB_NAMESPACE::ColumnFamilyOptions(), opt_string, + cf_options); env->ReleaseStringUTFChars(jopt_string, opt_string); @@ -5848,9 +5852,13 @@ jlong Java_org_rocksdb_DBOptions_getDBOptionsFromProps__Ljava_lang_String_2( return 0; } + const ROCKSDB_NAMESPACE::DBOptions base_options; auto* db_options = new ROCKSDB_NAMESPACE::DBOptions(); + ROCKSDB_NAMESPACE::ConfigOptions config_options(base_options); + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; ROCKSDB_NAMESPACE::Status status = ROCKSDB_NAMESPACE::GetDBOptionsFromString( - ROCKSDB_NAMESPACE::DBOptions(), opt_string, db_options); + config_options, base_options, opt_string, db_options); env->ReleaseStringUTFChars(jopt_string, opt_string); diff --git a/options/options_helper.cc b/options/options_helper.cc index dcc4cda70..9c320be28 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -658,18 +658,6 @@ Status GetStringFromCompressionType(std::string* compression_str, } } -Status GetColumnFamilyOptionsFromMap( - const ColumnFamilyOptions& base_options, - const std::unordered_map& opts_map, - ColumnFamilyOptions* new_options, bool input_strings_escaped, - bool ignore_unknown_options) { - ConfigOptions config_options; - config_options.ignore_unknown_options = ignore_unknown_options; - config_options.input_strings_escaped = input_strings_escaped; - return GetColumnFamilyOptionsFromMap(config_options, base_options, opts_map, - new_options); -} - Status GetColumnFamilyOptionsFromMap( const ConfigOptions& config_options, const ColumnFamilyOptions& base_options, @@ -691,17 +679,6 @@ Status GetColumnFamilyOptionsFromMap( } } -Status GetColumnFamilyOptionsFromString( - const ColumnFamilyOptions& base_options, - const std::string& opts_str, - ColumnFamilyOptions* new_options) { - ConfigOptions config_options; - config_options.input_strings_escaped = false; - config_options.ignore_unknown_options = false; - return GetColumnFamilyOptionsFromString(config_options, base_options, - opts_str, new_options); -} - Status GetColumnFamilyOptionsFromString(const ConfigOptions& config_options, const ColumnFamilyOptions& base_options, const std::string& opts_str, @@ -716,18 +693,6 @@ Status GetColumnFamilyOptionsFromString(const ConfigOptions& config_options, new_options); } -Status GetDBOptionsFromMap( - const DBOptions& base_options, - const std::unordered_map& opts_map, - DBOptions* new_options, bool input_strings_escaped, - bool ignore_unknown_options) { - ConfigOptions config_options(base_options); - config_options.input_strings_escaped = input_strings_escaped; - config_options.ignore_unknown_options = ignore_unknown_options; - return GetDBOptionsFromMap(config_options, base_options, opts_map, - new_options); -} - Status GetDBOptionsFromMap( const ConfigOptions& config_options, const DBOptions& base_options, const std::unordered_map& opts_map, @@ -746,17 +711,6 @@ Status GetDBOptionsFromMap( } } -Status GetDBOptionsFromString(const DBOptions& base_options, - const std::string& opts_str, - DBOptions* new_options) { - ConfigOptions config_options(base_options); - config_options.input_strings_escaped = false; - config_options.ignore_unknown_options = false; - - return GetDBOptionsFromString(config_options, base_options, opts_str, - new_options); -} - Status GetDBOptionsFromString(const ConfigOptions& config_options, const DBOptions& base_options, const std::string& opts_str, diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index de42e247a..020debf01 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -167,8 +167,13 @@ TEST_F(OptionsSettableTest, BlockBasedTableOptionsAllFieldsSettable) { kBbtoExcluded); // Need to update the option string if a new option is added. + ConfigOptions config_options; + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; + config_options.invoke_prepare_options = false; + config_options.ignore_unsupported_options = false; ASSERT_OK(GetBlockBasedTableOptionsFromString( - *bbto, + config_options, *bbto, "cache_index_and_filter_blocks=1;" "cache_index_and_filter_blocks_with_high_priority=true;" "metadata_cache_options={top_level_index_pinning=kFallback;" @@ -273,8 +278,11 @@ TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) { FillWithSpecialChar(new_options_ptr, sizeof(DBOptions), kDBOptionsExcluded); // Need to update the option string if a new option is added. + ConfigOptions config_options(*options); + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; ASSERT_OK( - GetDBOptionsFromString(*options, + GetDBOptionsFromString(config_options, *options, "wal_bytes_per_sync=4295048118;" "delete_obsolete_files_period_micros=4294967758;" "WAL_ttl_seconds=4295008036;" @@ -461,8 +469,11 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { kColumnFamilyOptionsExcluded); // Need to update the option string if a new option is added. + ConfigOptions config_options; + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; ASSERT_OK(GetColumnFamilyOptionsFromString( - *options, + config_options, *options, "compaction_filter_factory=mpudlojcujCompactionFilterFactory;" "table_factory=PlainTable;" "prefix_extractor=rocksdb.CappedPrefix.13;" diff --git a/options/options_test.cc b/options/options_test.cc index 4ac9a7c3f..481259a9e 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -811,9 +811,11 @@ TEST_F(OptionsTest, OldInterfaceTest) { ColumnFamilyOptions base_cf_opt; ColumnFamilyOptions new_cf_opt; ConfigOptions exact; - + ConfigOptions cf_config_options; + cf_config_options.input_strings_escaped = false; + cf_config_options.ignore_unknown_options = false; ASSERT_OK(GetColumnFamilyOptionsFromString( - base_cf_opt, + cf_config_options, base_cf_opt, "write_buffer_size=18;prefix_extractor=capped:8;" "arena_block_size=19", &new_cf_opt)); @@ -824,7 +826,7 @@ TEST_F(OptionsTest, OldInterfaceTest) { // And with a bad option ASSERT_NOK(GetColumnFamilyOptionsFromString( - base_cf_opt, + cf_config_options, base_cf_opt, "write_buffer_size=10;max_write_buffer_number=16;" "block_based_table_factory={xx_block_size=4;}", &new_cf_opt)); @@ -836,15 +838,17 @@ TEST_F(OptionsTest, OldInterfaceTest) { {"max_write_buffer_number", "2"}, {"min_write_buffer_number_to_merge", "3"}, }; - ASSERT_OK( - GetColumnFamilyOptionsFromMap(base_cf_opt, cf_options_map, &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); cf_options_map["unknown_option"] = "1"; - ASSERT_NOK( - GetColumnFamilyOptionsFromMap(base_cf_opt, cf_options_map, &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); ASSERT_OK( RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); - ASSERT_OK(GetColumnFamilyOptionsFromMap(base_cf_opt, cf_options_map, - &new_cf_opt, true, true)); + cf_config_options.input_strings_escaped = true; + cf_config_options.ignore_unknown_options = true; + ASSERT_OK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); DBOptions base_db_opt; DBOptions new_db_opt; @@ -857,7 +861,12 @@ TEST_F(OptionsTest, OldInterfaceTest) { {"verify_sst_unique_id_in_manifest", "true"}, {"max_open_files", "32"}, }; - ASSERT_OK(GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt)); + + ConfigOptions db_config_options(base_db_opt); + db_config_options.input_strings_escaped = false; + db_config_options.ignore_unknown_options = false; + ASSERT_OK(GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt)); ASSERT_EQ(new_db_opt.create_if_missing, false); ASSERT_EQ(new_db_opt.create_missing_column_families, true); ASSERT_EQ(new_db_opt.error_if_exists, false); @@ -866,23 +875,28 @@ TEST_F(OptionsTest, OldInterfaceTest) { ASSERT_EQ(new_db_opt.verify_sst_unique_id_in_manifest, true); ASSERT_EQ(new_db_opt.max_open_files, 32); db_options_map["unknown_option"] = "1"; - Status s = GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt); + Status s = GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt); ASSERT_NOK(s); ASSERT_TRUE(s.IsInvalidArgument()); ASSERT_OK( RocksDBOptionsParser::VerifyDBOptions(exact, base_db_opt, new_db_opt)); - ASSERT_OK(GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt, true, - true)); + db_config_options.input_strings_escaped = true; + db_config_options.ignore_unknown_options = true; + ASSERT_OK(GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt)); + db_config_options.input_strings_escaped = false; + db_config_options.ignore_unknown_options = false; ASSERT_OK(GetDBOptionsFromString( - base_db_opt, + db_config_options, base_db_opt, "create_if_missing=false;error_if_exists=false;max_open_files=42;", &new_db_opt)); ASSERT_EQ(new_db_opt.create_if_missing, false); ASSERT_EQ(new_db_opt.error_if_exists, false); ASSERT_EQ(new_db_opt.max_open_files, 42); s = GetDBOptionsFromString( - base_db_opt, + db_config_options, base_db_opt, "create_if_missing=false;error_if_exists=false;max_open_files=42;" "unknown_option=1;", &new_db_opt); @@ -2355,8 +2369,11 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { ColumnFamilyOptions base_cf_opt; ColumnFamilyOptions new_cf_opt; - ASSERT_OK(GetColumnFamilyOptionsFromMap( - base_cf_opt, cf_options_map, &new_cf_opt)); + ConfigOptions cf_config_options; + cf_config_options.ignore_unknown_options = false; + cf_config_options.input_strings_escaped = false; + ASSERT_OK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 1U); ASSERT_EQ(new_cf_opt.max_write_buffer_number, 2); ASSERT_EQ(new_cf_opt.min_write_buffer_number_to_merge, 3); @@ -2444,8 +2461,8 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { ASSERT_EQ(new_cf_opt.bottommost_temperature, Temperature::kWarm); cf_options_map["write_buffer_size"] = "hello"; - ASSERT_NOK(GetColumnFamilyOptionsFromMap( - base_cf_opt, cf_options_map, &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); ConfigOptions exact, loose; exact.sanity_level = ConfigOptions::kSanityLevelExactMatch; loose.sanity_level = ConfigOptions::kSanityLevelLooselyCompatible; @@ -2453,18 +2470,18 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); cf_options_map["write_buffer_size"] = "1"; - ASSERT_OK(GetColumnFamilyOptionsFromMap( - base_cf_opt, cf_options_map, &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); cf_options_map["unknown_option"] = "1"; - ASSERT_NOK(GetColumnFamilyOptionsFromMap( - base_cf_opt, cf_options_map, &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); - ASSERT_OK(GetColumnFamilyOptionsFromMap(base_cf_opt, cf_options_map, - &new_cf_opt, - false, /* input_strings_escaped */ - true /* ignore_unknown_options */)); + cf_config_options.input_strings_escaped = false; + cf_config_options.ignore_unknown_options = true; + ASSERT_OK(GetColumnFamilyOptionsFromMap(cf_config_options, base_cf_opt, + cf_options_map, &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions( loose, base_cf_opt, new_cf_opt, nullptr /* new_opt_map */)); ASSERT_NOK(RocksDBOptionsParser::VerifyCFOptions( @@ -2472,7 +2489,11 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { DBOptions base_db_opt; DBOptions new_db_opt; - ASSERT_OK(GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt)); + ConfigOptions db_config_options(base_db_opt); + db_config_options.input_strings_escaped = false; + db_config_options.ignore_unknown_options = false; + ASSERT_OK(GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt)); ASSERT_EQ(new_db_opt.create_if_missing, false); ASSERT_EQ(new_db_opt.create_missing_column_families, true); ASSERT_EQ(new_db_opt.error_if_exists, false); @@ -2515,18 +2536,21 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { ASSERT_EQ(new_db_opt.strict_bytes_per_sync, true); db_options_map["max_open_files"] = "hello"; - ASSERT_NOK(GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt)); + ASSERT_NOK(GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(exact, base_db_opt, new_db_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(loose, base_db_opt, new_db_opt)); // unknow options should fail parsing without ignore_unknown_options = true db_options_map["unknown_db_option"] = "1"; - ASSERT_NOK(GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt)); + ASSERT_NOK(GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(exact, base_db_opt, new_db_opt)); - ASSERT_OK(GetDBOptionsFromMap(base_db_opt, db_options_map, &new_db_opt, - false, /* input_strings_escaped */ - true /* ignore_unknown_options */)); + db_config_options.input_strings_escaped = false; + db_config_options.ignore_unknown_options = true; + ASSERT_OK(GetDBOptionsFromMap(db_config_options, base_db_opt, db_options_map, + &new_db_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(loose, base_db_opt, new_db_opt)); ASSERT_NOK(RocksDBOptionsParser::VerifyDBOptions(exact, base_db_opt, new_db_opt)); } @@ -2535,33 +2559,38 @@ TEST_F(OptionsOldApiTest, GetColumnFamilyOptionsFromStringTest) { ColumnFamilyOptions base_cf_opt; ColumnFamilyOptions new_cf_opt; base_cf_opt.table_factory.reset(); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, "", &new_cf_opt)); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=5", &new_cf_opt)); + ConfigOptions config_options; + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; + ASSERT_OK(GetColumnFamilyOptionsFromString(config_options, base_cf_opt, "", + &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, "write_buffer_size=5", &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 5U); ASSERT_TRUE(new_cf_opt.table_factory == nullptr); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=6;", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, "write_buffer_size=6;", &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 6U); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - " write_buffer_size = 7 ", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, " write_buffer_size = 7 ", &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 7U); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - " write_buffer_size = 8 ; ", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, " write_buffer_size = 8 ; ", &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 8U); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=9;max_write_buffer_number=10", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=9;max_write_buffer_number=10", &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 9U); ASSERT_EQ(new_cf_opt.max_write_buffer_number, 10); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=11; max_write_buffer_number = 12 ;", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=11; max_write_buffer_number = 12 ;", &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 11U); ASSERT_EQ(new_cf_opt.max_write_buffer_number, 12); // Wrong name "max_write_buffer_number_" - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=13;max_write_buffer_number_=14;", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=13;max_write_buffer_number_=14;", &new_cf_opt)); ConfigOptions exact; exact.sanity_level = ConfigOptions::kSanityLevelExactMatch; ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); @@ -2574,30 +2603,34 @@ TEST_F(OptionsOldApiTest, GetColumnFamilyOptionsFromStringTest) { std::unique_ptr* /*guard*/, std::string* /* errmsg */) { return ReverseBytewiseComparator(); }); - ASSERT_OK(GetColumnFamilyOptionsFromString( - base_cf_opt, "comparator=" + kCompName + ";", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString(config_options, base_cf_opt, + "comparator=" + kCompName + ";", + &new_cf_opt)); ASSERT_EQ(new_cf_opt.comparator, ReverseBytewiseComparator()); // MergeOperator from object registry std::unique_ptr bxo(new BytesXOROperator()); std::string kMoName = bxo->Name(); - ASSERT_OK(GetColumnFamilyOptionsFromString( - base_cf_opt, "merge_operator=" + kMoName + ";", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString(config_options, base_cf_opt, + "merge_operator=" + kMoName + ";", + &new_cf_opt)); ASSERT_EQ(kMoName, std::string(new_cf_opt.merge_operator->Name())); // Wrong key/value pair - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=13;max_write_buffer_number;", &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=13;max_write_buffer_number;", &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); // Error Paring value - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=13;max_write_buffer_number=;", &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=13;max_write_buffer_number=;", &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); // Missing option name - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=13; =100;", &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, "write_buffer_size=13; =100;", &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); const uint64_t kilo = 1024UL; @@ -2607,17 +2640,17 @@ TEST_F(OptionsOldApiTest, GetColumnFamilyOptionsFromStringTest) { // Units (k) ASSERT_OK(GetColumnFamilyOptionsFromString( - base_cf_opt, "max_write_buffer_number=15K", &new_cf_opt)); + config_options, base_cf_opt, "max_write_buffer_number=15K", &new_cf_opt)); ASSERT_EQ(new_cf_opt.max_write_buffer_number, 15 * kilo); // Units (m) - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "max_write_buffer_number=16m;inplace_update_num_locks=17M", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "max_write_buffer_number=16m;inplace_update_num_locks=17M", &new_cf_opt)); ASSERT_EQ(new_cf_opt.max_write_buffer_number, 16 * mega); ASSERT_EQ(new_cf_opt.inplace_update_num_locks, 17u * mega); // Units (g) ASSERT_OK(GetColumnFamilyOptionsFromString( - base_cf_opt, + config_options, base_cf_opt, "write_buffer_size=18g;prefix_extractor=capped:8;" "arena_block_size=19G", &new_cf_opt)); @@ -2628,107 +2661,119 @@ TEST_F(OptionsOldApiTest, GetColumnFamilyOptionsFromStringTest) { ASSERT_EQ(new_cf_opt.prefix_extractor->AsString(), "rocksdb.CappedPrefix.8"); // Units (t) - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=20t;arena_block_size=21T", &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, "write_buffer_size=20t;arena_block_size=21T", + &new_cf_opt)); ASSERT_EQ(new_cf_opt.write_buffer_size, 20 * tera); ASSERT_EQ(new_cf_opt.arena_block_size, 21 * tera); // Nested block based table options // Empty - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={};arena_block_size=1024", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={};arena_block_size=1024", + &new_cf_opt)); ASSERT_TRUE(new_cf_opt.table_factory != nullptr); // Non-empty - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={block_cache=1M;block_size=4;};" - "arena_block_size=1024", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={block_cache=1M;block_size=4;};" + "arena_block_size=1024", + &new_cf_opt)); ASSERT_TRUE(new_cf_opt.table_factory != nullptr); // Last one - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={block_cache=1M;block_size=4;}", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={block_cache=1M;block_size=4;}", + &new_cf_opt)); ASSERT_TRUE(new_cf_opt.table_factory != nullptr); // Mismatch curly braces - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={{{block_size=4;};" - "arena_block_size=1024", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={{{block_size=4;};" + "arena_block_size=1024", + &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); // Unexpected chars after closing curly brace - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={block_size=4;}};" - "arena_block_size=1024", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={block_size=4;}};" + "arena_block_size=1024", + &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={block_size=4;}xdfa;" - "arena_block_size=1024", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={block_size=4;}xdfa;" + "arena_block_size=1024", + &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={block_size=4;}xdfa", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={block_size=4;}xdfa", + &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); // Invalid block based table option - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "block_based_table_factory={xx_block_size=4;}", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "block_based_table_factory={xx_block_size=4;}", + &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "optimize_filters_for_hits=true", - &new_cf_opt)); - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "optimize_filters_for_hits=false", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString(config_options, base_cf_opt, + "optimize_filters_for_hits=true", + &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString(config_options, base_cf_opt, + "optimize_filters_for_hits=false", + &new_cf_opt)); - ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt, - "optimize_filters_for_hits=junk", - &new_cf_opt)); + ASSERT_NOK(GetColumnFamilyOptionsFromString(config_options, base_cf_opt, + "optimize_filters_for_hits=junk", + &new_cf_opt)); ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(exact, base_cf_opt, new_cf_opt)); // Nested plain table options // Empty - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "plain_table_factory={};arena_block_size=1024", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "plain_table_factory={};arena_block_size=1024", + &new_cf_opt)); ASSERT_TRUE(new_cf_opt.table_factory != nullptr); ASSERT_EQ(std::string(new_cf_opt.table_factory->Name()), "PlainTable"); // Non-empty - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "plain_table_factory={user_key_len=66;bloom_bits_per_key=20;};" - "arena_block_size=1024", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "plain_table_factory={user_key_len=66;bloom_bits_per_key=20;};" + "arena_block_size=1024", + &new_cf_opt)); ASSERT_TRUE(new_cf_opt.table_factory != nullptr); ASSERT_EQ(std::string(new_cf_opt.table_factory->Name()), "PlainTable"); // memtable factory - ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, - "write_buffer_size=10;max_write_buffer_number=16;" - "memtable=skip_list:10;arena_block_size=1024", - &new_cf_opt)); + ASSERT_OK(GetColumnFamilyOptionsFromString( + config_options, base_cf_opt, + "write_buffer_size=10;max_write_buffer_number=16;" + "memtable=skip_list:10;arena_block_size=1024", + &new_cf_opt)); ASSERT_TRUE(new_cf_opt.memtable_factory != nullptr); ASSERT_TRUE(new_cf_opt.memtable_factory->IsInstanceOf("SkipListFactory")); // blob cache ASSERT_OK(GetColumnFamilyOptionsFromString( - base_cf_opt, + config_options, base_cf_opt, "blob_cache={capacity=1M;num_shard_bits=4;" "strict_capacity_limit=true;high_pri_pool_ratio=0.5;};", &new_cf_opt)); @@ -2832,9 +2877,15 @@ TEST_F(OptionsTest, SliceTransformCreateFromString) { TEST_F(OptionsOldApiTest, GetBlockBasedTableOptionsFromString) { BlockBasedTableOptions table_opt; BlockBasedTableOptions new_opt; + ConfigOptions config_options; + config_options.input_strings_escaped = false; + config_options.ignore_unknown_options = false; + config_options.invoke_prepare_options = false; + config_options.ignore_unsupported_options = false; + // make sure default values are overwritten by something else ASSERT_OK(GetBlockBasedTableOptionsFromString( - table_opt, + config_options, table_opt, "cache_index_and_filter_blocks=1;index_type=kHashSearch;" "checksum=kxxHash;no_block_cache=1;" "block_cache=1M;block_cache_compressed=1k;block_size=1024;" @@ -2860,54 +2911,57 @@ TEST_F(OptionsOldApiTest, GetBlockBasedTableOptionsFromString) { EXPECT_EQ(bfp->GetWholeBitsPerKey(), 5); // unknown option - ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt, - "cache_index_and_filter_blocks=1;index_type=kBinarySearch;" - "bad_option=1", - &new_opt)); + ASSERT_NOK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, + "cache_index_and_filter_blocks=1;index_type=kBinarySearch;" + "bad_option=1", + &new_opt)); ASSERT_EQ(static_cast(table_opt.cache_index_and_filter_blocks), new_opt.cache_index_and_filter_blocks); ASSERT_EQ(table_opt.index_type, new_opt.index_type); // unrecognized index type - ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt, - "cache_index_and_filter_blocks=1;index_type=kBinarySearchXX", - &new_opt)); + ASSERT_NOK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, + "cache_index_and_filter_blocks=1;index_type=kBinarySearchXX", &new_opt)); ASSERT_EQ(table_opt.cache_index_and_filter_blocks, new_opt.cache_index_and_filter_blocks); ASSERT_EQ(table_opt.index_type, new_opt.index_type); // unrecognized checksum type - ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt, - "cache_index_and_filter_blocks=1;checksum=kxxHashXX", - &new_opt)); + ASSERT_NOK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, + "cache_index_and_filter_blocks=1;checksum=kxxHashXX", &new_opt)); ASSERT_EQ(table_opt.cache_index_and_filter_blocks, new_opt.cache_index_and_filter_blocks); ASSERT_EQ(table_opt.index_type, new_opt.index_type); // unrecognized filter policy name - ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt, - "cache_index_and_filter_blocks=1;" - "filter_policy=bloomfilterxx:4:true", - &new_opt)); + ASSERT_NOK( + GetBlockBasedTableOptionsFromString(config_options, table_opt, + "cache_index_and_filter_blocks=1;" + "filter_policy=bloomfilterxx:4:true", + &new_opt)); ASSERT_EQ(table_opt.cache_index_and_filter_blocks, new_opt.cache_index_and_filter_blocks); ASSERT_EQ(table_opt.filter_policy, new_opt.filter_policy); // Used to be rejected, now accepted ASSERT_OK(GetBlockBasedTableOptionsFromString( - table_opt, "filter_policy=bloomfilter:4", &new_opt)); + config_options, table_opt, "filter_policy=bloomfilter:4", &new_opt)); bfp = dynamic_cast(new_opt.filter_policy.get()); EXPECT_EQ(bfp->GetMillibitsPerKey(), 4000); EXPECT_EQ(bfp->GetWholeBitsPerKey(), 4); // Check block cache options are overwritten when specified // in new format as a struct. - ASSERT_OK(GetBlockBasedTableOptionsFromString(table_opt, - "block_cache={capacity=1M;num_shard_bits=4;" - "strict_capacity_limit=true;high_pri_pool_ratio=0.5;};" - "block_cache_compressed={capacity=1M;num_shard_bits=4;" - "strict_capacity_limit=true;high_pri_pool_ratio=0.5;}", - &new_opt)); + ASSERT_OK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, + "block_cache={capacity=1M;num_shard_bits=4;" + "strict_capacity_limit=true;high_pri_pool_ratio=0.5;};" + "block_cache_compressed={capacity=1M;num_shard_bits=4;" + "strict_capacity_limit=true;high_pri_pool_ratio=0.5;}", + &new_opt)); ASSERT_TRUE(new_opt.block_cache != nullptr); ASSERT_EQ(new_opt.block_cache->GetCapacity(), 1024UL*1024UL); ASSERT_EQ(std::dynamic_pointer_cast(new_opt.block_cache) @@ -2919,10 +2973,11 @@ TEST_F(OptionsOldApiTest, GetBlockBasedTableOptionsFromString) { // Set only block cache capacity. Check other values are // reset to default values. - ASSERT_OK(GetBlockBasedTableOptionsFromString(table_opt, - "block_cache={capacity=2M};" - "block_cache_compressed={capacity=2M}", - &new_opt)); + ASSERT_OK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, + "block_cache={capacity=2M};" + "block_cache_compressed={capacity=2M}", + &new_opt)); ASSERT_TRUE(new_opt.block_cache != nullptr); ASSERT_EQ(new_opt.block_cache->GetCapacity(), 2*1024UL*1024UL); // Default values @@ -2936,7 +2991,7 @@ TEST_F(OptionsOldApiTest, GetBlockBasedTableOptionsFromString) { // Set couple of block cache options. ASSERT_OK(GetBlockBasedTableOptionsFromString( - table_opt, + config_options, table_opt, "block_cache={num_shard_bits=5;high_pri_pool_ratio=0.5;};" "block_cache_compressed={num_shard_bits=5;" "high_pri_pool_ratio=0.0;}", @@ -2950,12 +3005,13 @@ TEST_F(OptionsOldApiTest, GetBlockBasedTableOptionsFromString) { new_opt.block_cache)->GetHighPriPoolRatio(), 0.5); // Set couple of block cache options. - ASSERT_OK(GetBlockBasedTableOptionsFromString(table_opt, - "block_cache={capacity=1M;num_shard_bits=4;" - "strict_capacity_limit=true;};" - "block_cache_compressed={capacity=1M;num_shard_bits=4;" - "strict_capacity_limit=true;}", - &new_opt)); + ASSERT_OK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, + "block_cache={capacity=1M;num_shard_bits=4;" + "strict_capacity_limit=true;};" + "block_cache_compressed={capacity=1M;num_shard_bits=4;" + "strict_capacity_limit=true;}", + &new_opt)); ASSERT_TRUE(new_opt.block_cache != nullptr); ASSERT_EQ(new_opt.block_cache->GetCapacity(), 1024UL*1024UL); ASSERT_EQ(std::dynamic_pointer_cast(new_opt.block_cache) @@ -2971,11 +3027,16 @@ TEST_F(OptionsOldApiTest, GetPlainTableOptionsFromString) { PlainTableOptions table_opt; PlainTableOptions new_opt; // make sure default values are overwritten by something else - ASSERT_OK(GetPlainTableOptionsFromString(table_opt, - "user_key_len=66;bloom_bits_per_key=20;hash_table_ratio=0.5;" - "index_sparseness=8;huge_page_tlb_size=4;encoding_type=kPrefix;" - "full_scan_mode=true;store_index_in_file=true", - &new_opt)); + ConfigOptions config_options_from_string; + config_options_from_string.input_strings_escaped = false; + config_options_from_string.ignore_unknown_options = false; + config_options_from_string.invoke_prepare_options = false; + ASSERT_OK(GetPlainTableOptionsFromString( + config_options_from_string, table_opt, + "user_key_len=66;bloom_bits_per_key=20;hash_table_ratio=0.5;" + "index_sparseness=8;huge_page_tlb_size=4;encoding_type=kPrefix;" + "full_scan_mode=true;store_index_in_file=true", + &new_opt)); ASSERT_EQ(new_opt.user_key_len, 66u); ASSERT_EQ(new_opt.bloom_bits_per_key, 20); ASSERT_EQ(new_opt.hash_table_ratio, 0.5); @@ -2988,22 +3049,28 @@ TEST_F(OptionsOldApiTest, GetPlainTableOptionsFromString) { std::unordered_map opt_map; ASSERT_OK(StringToMap( "user_key_len=55;bloom_bits_per_key=10;huge_page_tlb_size=8;", &opt_map)); - ASSERT_OK(GetPlainTableOptionsFromMap(table_opt, opt_map, &new_opt)); + ConfigOptions config_options_from_map; + config_options_from_map.input_strings_escaped = false; + config_options_from_map.ignore_unknown_options = false; + ASSERT_OK(GetPlainTableOptionsFromMap(config_options_from_map, table_opt, + opt_map, &new_opt)); ASSERT_EQ(new_opt.user_key_len, 55u); ASSERT_EQ(new_opt.bloom_bits_per_key, 10); ASSERT_EQ(new_opt.huge_page_tlb_size, 8); // unknown option - ASSERT_NOK(GetPlainTableOptionsFromString(table_opt, - "user_key_len=66;bloom_bits_per_key=20;hash_table_ratio=0.5;" - "bad_option=1", - &new_opt)); + ASSERT_NOK(GetPlainTableOptionsFromString( + config_options_from_string, table_opt, + "user_key_len=66;bloom_bits_per_key=20;hash_table_ratio=0.5;" + "bad_option=1", + &new_opt)); // unrecognized EncodingType - ASSERT_NOK(GetPlainTableOptionsFromString(table_opt, - "user_key_len=66;bloom_bits_per_key=20;hash_table_ratio=0.5;" - "encoding_type=kPrefixXX", - &new_opt)); + ASSERT_NOK(GetPlainTableOptionsFromString( + config_options_from_string, table_opt, + "user_key_len=66;bloom_bits_per_key=20;hash_table_ratio=0.5;" + "encoding_type=kPrefixXX", + &new_opt)); } TEST_F(OptionsOldApiTest, GetOptionsFromStringTest) { @@ -3085,10 +3152,15 @@ TEST_F(OptionsOldApiTest, DBOptionsSerialization) { // Phase 3: Set new_options from the derived string and expect // new_options == base_options - ASSERT_OK(GetDBOptionsFromString(DBOptions(), base_options_file_content, - &new_options)); - ConfigOptions config_options; - ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(config_options, base_options, new_options)); + const DBOptions base_db_options; + ConfigOptions db_config_options(base_db_options); + db_config_options.input_strings_escaped = false; + db_config_options.ignore_unknown_options = false; + ASSERT_OK(GetDBOptionsFromString(db_config_options, base_db_options, + base_options_file_content, &new_options)); + ConfigOptions verify_db_config_options; + ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(verify_db_config_options, + base_options, new_options)); } TEST_F(OptionsOldApiTest, ColumnFamilyOptionsSerialization) { @@ -3106,10 +3178,15 @@ TEST_F(OptionsOldApiTest, ColumnFamilyOptionsSerialization) { // Phase 3: Set new_opt from the derived string and expect // new_opt == base_opt - ASSERT_OK(GetColumnFamilyOptionsFromString( - ColumnFamilyOptions(), base_options_file_content, &new_opt)); - ConfigOptions config_options; - ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config_options, base_opt, new_opt)); + ConfigOptions cf_config_options; + cf_config_options.input_strings_escaped = false; + cf_config_options.ignore_unknown_options = false; + ASSERT_OK( + GetColumnFamilyOptionsFromString(cf_config_options, ColumnFamilyOptions(), + base_options_file_content, &new_opt)); + ConfigOptions verify_cf_config_options; + ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(verify_cf_config_options, + base_opt, new_opt)); if (base_opt.compaction_filter) { delete base_opt.compaction_filter; } diff --git a/table/block_based/block_based_table_factory.cc b/table/block_based/block_based_table_factory.cc index 9708db393..845f3a619 100644 --- a/table/block_based/block_based_table_factory.cc +++ b/table/block_based/block_based_table_factory.cc @@ -909,18 +909,6 @@ Status BlockBasedTableFactory::ParseOption(const ConfigOptions& config_options, return status; } -Status GetBlockBasedTableOptionsFromString( - const BlockBasedTableOptions& table_options, const std::string& opts_str, - BlockBasedTableOptions* new_table_options) { - ConfigOptions config_options; - config_options.input_strings_escaped = false; - config_options.ignore_unknown_options = false; - config_options.invoke_prepare_options = false; - config_options.ignore_unsupported_options = false; - - return GetBlockBasedTableOptionsFromString(config_options, table_options, - opts_str, new_table_options); -} Status GetBlockBasedTableOptionsFromString( const ConfigOptions& config_options, const BlockBasedTableOptions& table_options, const std::string& opts_str, @@ -940,20 +928,6 @@ Status GetBlockBasedTableOptionsFromString( } } -Status GetBlockBasedTableOptionsFromMap( - const BlockBasedTableOptions& table_options, - const std::unordered_map& opts_map, - BlockBasedTableOptions* new_table_options, bool input_strings_escaped, - bool ignore_unknown_options) { - ConfigOptions config_options; - config_options.input_strings_escaped = input_strings_escaped; - config_options.ignore_unknown_options = ignore_unknown_options; - config_options.invoke_prepare_options = false; - - return GetBlockBasedTableOptionsFromMap(config_options, table_options, - opts_map, new_table_options); -} - Status GetBlockBasedTableOptionsFromMap( const ConfigOptions& config_options, const BlockBasedTableOptions& table_options, diff --git a/table/plain/plain_table_factory.cc b/table/plain/plain_table_factory.cc index c1402d8ef..80aa9cb8e 100644 --- a/table/plain/plain_table_factory.cc +++ b/table/plain/plain_table_factory.cc @@ -122,17 +122,6 @@ std::string PlainTableFactory::GetPrintableOptions() const { return ret; } -Status GetPlainTableOptionsFromString(const PlainTableOptions& table_options, - const std::string& opts_str, - PlainTableOptions* new_table_options) { - ConfigOptions config_options; - config_options.input_strings_escaped = false; - config_options.ignore_unknown_options = false; - config_options.invoke_prepare_options = false; - return GetPlainTableOptionsFromString(config_options, table_options, opts_str, - new_table_options); -} - Status GetPlainTableOptionsFromString(const ConfigOptions& config_options, const PlainTableOptions& table_options, const std::string& opts_str, @@ -275,18 +264,6 @@ Status MemTableRepFactory::CreateFromString( return s; } -Status GetPlainTableOptionsFromMap( - const PlainTableOptions& table_options, - const std::unordered_map& opts_map, - PlainTableOptions* new_table_options, bool input_strings_escaped, - bool ignore_unknown_options) { - ConfigOptions config_options; - config_options.input_strings_escaped = input_strings_escaped; - config_options.ignore_unknown_options = ignore_unknown_options; - return GetPlainTableOptionsFromMap(config_options, table_options, opts_map, - new_table_options); -} - Status GetPlainTableOptionsFromMap( const ConfigOptions& config_options, const PlainTableOptions& table_options, const std::unordered_map& opts_map,