From 6eb317bb4ce72d5809d07b4d33be550f75db113c Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 25 Apr 2019 11:31:58 -0700 Subject: [PATCH] Option string/map/file can set env from object registry (#5237) Summary: - By providing the "env" field in any text-based options (i.e., string, map, or file), we can use `NewCustomObject` to deserialize the text value into an actual `Env` object. - Currently factory functions for `Env` registered with object registry should only return pointer to static `Env` objects. That's because `DBOptions::env` is a raw pointer so we cannot easily delegate cleanup. - Note I did not add `env` to `db_option_type_info`. It wasn't needed for (de)serialization, and I believe we don't want to do verification on `env`, even by checking name. That's because the user should be able to copy their DB from Linux to Windows, change envs, and not see an option verification error. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5237 Differential Revision: D15056360 Pulled By: siying fbshipit-source-id: 4b5f0b83297a5058f8949ec955dbf27d98d73d7e --- HISTORY.md | 5 ++--- include/rocksdb/utilities/options_util.h | 5 +++-- options/options_helper.cc | 18 +++++++++++++----- options/options_helper.h | 3 ++- options/options_test.cc | 19 ++++++++++++++++++- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index e2ec3265c..1efe5d8df 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,12 +2,11 @@ ## Unreleased ### New Features * Add an option `strict_bytes_per_sync` that causes a file-writing thread to block rather than exceed the limit on bytes pending writeback specified by `bytes_per_sync` or `wal_bytes_per_sync`. - -## Unreleased -### New Features * Improve range scan performance by avoiding per-key upper bound check in BlockBasedTableIterator. * Introduce Periodic Compaction for Level style compaction. Files are re-compacted periodically and put in the same level. * Block-based table index now contains exact highest key in the file, rather than an upper bound. This may improve Get() and iterator Seek() performance in some situations, especially when direct IO is enabled and block cache is disabled. A setting BlockBasedTableOptions::index_shortening is introduced to control this behavior. Set it to kShortenSeparatorsAndSuccessor to get the old behavior. +* When reading from option file/string/map, customized envs can be filled according to object registry. + ### Public API Change * Change the behavior of OptimizeForPointLookup(): move away from hash-based block-based-table index, and use whole key memtable filtering. * Change the behavior of OptimizeForSmallDb(): use a 16MB block cache, put index and filter blocks into it, and cost the memtable size to it. DBOptions.OptimizeForSmallDb() and ColumnFamilyOptions.OptimizeForSmallDb() start to take an optional cache object. diff --git a/include/rocksdb/utilities/options_util.h b/include/rocksdb/utilities/options_util.h index d97b394ea..a1d0fde2f 100644 --- a/include/rocksdb/utilities/options_util.h +++ b/include/rocksdb/utilities/options_util.h @@ -33,9 +33,10 @@ namespace rocksdb { // * merge_operator // * compaction_filter // -// User can also choose to load customized comparator and/or merge_operator -// through object registry: +// User can also choose to load customized comparator, env, and/or +// merge_operator through object registry: // * comparator needs to be registered through Registrar +// * env needs to be registered through Registrar // * merge operator needs to be registered through // Registrar>. // diff --git a/options/options_helper.cc b/options/options_helper.cc index 57a1f9392..b7781ff6d 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -247,6 +247,7 @@ std::unordered_map #ifndef ROCKSDB_LITE const std::string kNameComparator = "comparator"; +const std::string kNameEnv = "env"; const std::string kNameMergeOperator = "merge_operator"; template @@ -1179,6 +1180,14 @@ Status ParseDBOption(const std::string& name, if (name == "rate_limiter_bytes_per_sec") { new_options->rate_limiter.reset( NewGenericRateLimiter(static_cast(ParseUint64(value)))); + } else if (name == kNameEnv) { + // Currently `Env` can be deserialized from object registry only. + std::unique_ptr env_guard; + Env* env = NewCustomObject(value, &env_guard); + // Only support static env for now. + if (env != nullptr && !env_guard) { + new_options->env = env; + } } else { auto iter = db_options_type_info.find(name); if (iter == db_options_type_info.end()) { @@ -1388,7 +1397,6 @@ std::unordered_map OptionsHelper::db_options_type_info = { /* // not yet supported - Env* env; std::shared_ptr row_cache; std::shared_ptr delete_scheduler; std::shared_ptr info_log; @@ -1553,8 +1561,8 @@ std::unordered_map OptionVerificationType::kNormal, true, offsetof(struct MutableDBOptions, wal_bytes_per_sync)}}, {"strict_bytes_per_sync", - {offsetof(struct DBOptions, strict_bytes_per_sync), OptionType::kBoolean, - OptionVerificationType::kNormal, true, + {offsetof(struct DBOptions, strict_bytes_per_sync), + OptionType::kBoolean, OptionVerificationType::kNormal, true, offsetof(struct MutableDBOptions, strict_bytes_per_sync)}}, {"stats_dump_period_sec", {offsetof(struct DBOptions, stats_dump_period_sec), OptionType::kUInt, @@ -1639,8 +1647,8 @@ std::unordered_map {"avoid_unnecessary_blocking_io", {offsetof(struct DBOptions, avoid_unnecessary_blocking_io), OptionType::kBoolean, OptionVerificationType::kNormal, false, - offsetof(struct ImmutableDBOptions, avoid_unnecessary_blocking_io)}} - }; + offsetof(struct ImmutableDBOptions, avoid_unnecessary_blocking_io)}}, +}; std::unordered_map OptionsHelper::block_base_table_index_type_string_map = { diff --git a/options/options_helper.h b/options/options_helper.h index 004522018..c5463999b 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -81,7 +81,8 @@ enum class OptionType { kAccessHint, kInfoLogLevel, kLRUCacheOptions, - kUnknown + kEnv, + kUnknown, }; enum class OptionVerificationType { diff --git a/options/options_test.cc b/options/options_test.cc index 03686bac7..fbfee311b 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -759,6 +759,21 @@ TEST_F(OptionsTest, GetOptionsFromStringTest) { block_based_table_options.cache_index_and_filter_blocks = true; base_options.table_factory.reset( NewBlockBasedTableFactory(block_based_table_options)); + + // Register an Env with object registry. + const static char* kCustomEnvName = "CustomEnv"; + class CustomEnv : public EnvWrapper { + public: + explicit CustomEnv(Env* _target) : EnvWrapper(_target) {} + }; + + static Registrar test_reg_env( + kCustomEnvName, + [](const std::string& /*name*/, std::unique_ptr* /*env_guard*/) { + static CustomEnv env(Env::Default()); + return &env; + }); + ASSERT_OK(GetOptionsFromString( base_options, "write_buffer_size=10;max_write_buffer_number=16;" @@ -766,7 +781,7 @@ TEST_F(OptionsTest, GetOptionsFromStringTest) { "compression_opts=4:5:6;create_if_missing=true;max_open_files=1;" "bottommost_compression_opts=5:6:7;create_if_missing=true;max_open_files=" "1;" - "rate_limiter_bytes_per_sec=1024", + "rate_limiter_bytes_per_sec=1024;env=CustomEnv", &new_options)); ASSERT_EQ(new_options.compression_opts.window_bits, 4); @@ -795,6 +810,8 @@ TEST_F(OptionsTest, GetOptionsFromStringTest) { ASSERT_EQ(new_options.create_if_missing, true); ASSERT_EQ(new_options.max_open_files, 1); ASSERT_TRUE(new_options.rate_limiter.get() != nullptr); + std::unique_ptr env_guard; + ASSERT_EQ(NewCustomObject(kCustomEnvName, &env_guard), new_options.env); } TEST_F(OptionsTest, DBOptionsSerialization) {