Improve comment and bug fix for GetOptionsFromMap functions in convenience.h

Summary:
This diff improves the documentation for GetOptionsFromMap APIs and
fixes a bug in GetOptionsFromMap functions in convenience.h
where new_options will still be changed when the function
call is not successful.

Test Plan: options_test

Reviewers: IslamAbdelRahman, kradhakrishnan, andrewkr, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D61731

include/rocksdb/convenience.h

@@ -16,36 +16,243 @@
 namespace rocksdb {
 
 #ifndef ROCKSDB_LITE
-// Take a map of option name and option value, apply them into the
+// The following set of functions provide a way to construct RocksDB Options
-// base_options, and return the new options as a result.
+// from a string or a string-to-string map.  Here're the general rule of
+// setting option values from strings by type.  Some RocksDB types are also
+// supported in these APIs.  Please refer to the comment of the function itself
+// to find more information about how to config those RocksDB types.
 //
-// If input_strings_escaped is set to true, then each escaped characters
+// * Strings:
-// prefixed by '\' in the values of the opts_map will be further
+//   Strings will be used as values directly without any truncating or
-// converted back to the raw string before assigning to the associated
+//   trimming.
-// options.
+//
+// * Booleans:
+//   - "true" or "1" => true
+//   - "false" or "0" => false.
+//   [Example]:
+//   - {"optimize_filters_for_hits", "1"} in GetColumnFamilyOptionsFromMap, or
+//   - "optimize_filters_for_hits=true" in GetColumnFamilyOptionsFromString.
+//
+// * Integers:
+//   Integers are converted directly from string, in addtion to the following
+//   units that we support:
+//   - 'k' or 'K' => 2^10
+//   - 'm' or 'M' => 2^20
+//   - 'g' or 'G' => 2^30
+//   - 't' or 'T' => 2^40  // only for unsigned int with sufficient bits.
+//   [Example]:
+//   - {"arena_block_size", "19G"} in GetColumnFamilyOptionsFromMap, or
+//   - "arena_block_size=19G" in GetColumnFamilyOptionsFromString.
+//
+// * Doubles / Floating Points:
+//   Doubles / Floating Points are converted directly from string.  Note that
+//   currently we do not support units.
+//   [Example]:
+//   - {"hard_rate_limit", "2.1"} in GetColumnFamilyOptionsFromMap, or
+//   - "hard_rate_limit=2.1" in GetColumnFamilyOptionsFromString.
+// * Array / Vectors:
+//   An array is specified by a list of values, where ':' is used as
+//   the delimiter to separate each value.
+//   [Example]:
+//   - {"compression_per_level", "kNoCompression:kSnappyCompression"}
+//     in GetColumnFamilyOptionsFromMap, or
+//   - "compression_per_level=kNoCompression:kSnappyCompression" in
+//     GetColumnFamilyOptionsFromMapString
+// * Enums:
+//   The valid values of each enum are identical to the names of its constants.
+//   [Example]:
+//   - CompressionType: valid values are "kNoCompression",
+//     "kSnappyCompression", "kZlibCompression", "kBZip2Compression", ...
+//   - CompactionStyle: valid values are "kCompactionStyleLevel",
+//     "kCompactionStyleUniversal", "kCompactionStyleFIFO", and
+//     "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".
+//
+// Below are the instructions of how to config some non-primitive-typed
+// options in ColumnFOptions:
+//
+// * table_factory:
+//   table_factory can be configured using our custom nested-option syntax.
+//
+//   {option_a=value_a; option_b=value_b; option_c=value_c; ... }
+//
+//   A nested option is enclosed by two curly braces, within which there are
+//   multiple option assignments.  Each assignment is of the form
+//   "variable_name=value;".
+//
+//   Currently we support the following types of TableFactory:
+//   - BlockBasedTableFactory:
+//     Use name "block_based_table_factory" to initialize table_factory with
+//     BlockBasedTableFactory.  Its BlockBasedTableFactoryOptions can be
+//     configured using the nested-option syntax.
+//     [Example]:
+//     * {"block_based_table_factory", "{block_cache=1M;block_size=4k;}"}
+//       is equivalent to assigning table_factory with a BlockBasedTableFactory
+//       that has 1M LRU block-cache with block size equals to 4k:
+//         ColumnFamilyOptions cf_opt;
+//         BlockBasedTableOptions blk_opt;
+//         blk_opt.block_cache = NewLRUCache(1 * 1024 * 1024);
+//         blk_opt.block_size = 4 * 1024;
+//         cf_opt.table_factory.reset(NewBlockBasedTableFactory(blk_opt));
+//   - PlainTableFactory:
+//     Use name "plain_table_factory" to initialize table_factory with
+//     PlainTableFactory.  Its PlainTableFactoryOptions can be configured using
+//     the nested-option syntax.
+//     [Example]:
+//     * {"plain_table_factory", "{user_key_len=66;bloom_bits_per_key=20;}"}
+//
+// * memtable_factory:
+//   Use "memtable" to config memtable_factory.  Here are the supported
+//   memtable factories:
+//   - SkipList:
+//     Pass "skip_list:<lookahead>" to config memtable to use SkipList,
+//     or simply "skip_list" to use the default SkipList.
+//     [Example]:
+//     * {"memtable", "skip_list:5"} is equivalent to setting
+//       memtable to SkipListFactory(5).
+//   - PrefixHash:
+//     Pass "prfix_hash:<hash_bucket_count>" to config memtable
+//     to use PrefixHash, or simply "prefix_hash" to use the default
+//     PrefixHash.
+//     [Example]:
+//     * {"memtable", "prefix_hash:1000"} is equivalent to setting
+//       memtable to NewHashSkipListRepFactory(hash_bucket_count).
+//   - HashLinkedList:
+//     Pass "hash_linkedlist:<hash_bucket_count>" to config memtable
+//     to use HashLinkedList, or simply "hash_linkedlist" to use the default
+//     HashLinkedList.
+//     [Example]:
+//     * {"memtable", "hash_linkedlist:1000"} is equivalent to
+//       setting memtable to NewHashLinkListRepFactory(1000).
+//   - VectorRepFactory:
+//     Pass "vector:<count>" to config memtable to use VectorRepFactory,
+//     or simply "vector" to use the default Vector memtable.
+//     [Example]:
+//     * {"memtable", "vector:1024"} is equivalent to setting memtable
+//       to VectorRepFactory(1024).
+//   - HashCuckooRepFactory:
+//     Pass "cuckoo:<write_buffer_size>" to use HashCuckooRepFactory with the
+//     specified write buffer size, or simply "cuckoo" to use the default
+//     HashCuckooRepFactory.
+//     [Example]:
+//     * {"memtable", "cuckoo:1024"} is equivalent to setting memtable
+//       to NewHashCuckooRepFactory(1024).
+//
+//  * compression_opts:
+//    Use "compression_opts" to config compression_opts.  The value format
+//    is of the form "<window_bits>:<level>:<strategy>:<max_dict_bytes>".
+//    [Example]:
+//    * {"compression_opts", "4:5:6:7"} is equivalent to setting:
+//        ColumnFamilyOptions cf_opt;
+//        cf_opt.compression_opts.window_bits = 4;
+//        cf_opt.compression_opts.level = 5;
+//        cf_opt.compression_opts.strategy = 6;
+//        cf_opt.compression_opts.max_dict_bytes = 7;
+//
+// @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"
+//     should be set.
+// @param new_options the resulting options based on "base_options" with the
+//     change specified in "opts_map".
+// @param input_strings_escaped when set to true, each escaped characters
+//     prefixed by '\' in the values of the opts_map will be further converted
+//     back to the raw string before assigning to the associated options.
+// @return Status::OK() on success.  Otherwise, a non-ok status indicating
+//     error will be returned, and "new_options" will be set to "base_options".
 Status GetColumnFamilyOptionsFromMap(
     const ColumnFamilyOptions& base_options,
     const std::unordered_map<std::string, std::string>& opts_map,
     ColumnFamilyOptions* new_options, bool input_strings_escaped = false);
 
-// Take a map of option name and option value, apply them into the
+// Take a default DBOptions "base_options" in addition to a
-// base_options, and return the new options as a result.
+// 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:
+//
+// * rate_limiter_bytes_per_sec:
+//   RateLimiter can be configured directly by specifying its bytes_per_sec.
+//   [Example]:
+//   - Passing {"rate_limiter_bytes_per_sec", "1024"} is equivalent to
+//     passing NewGenericRateLimiter(1024) to rate_limiter_bytes_per_sec.
 //
-// If input_strings_escaped is set to true, then each escaped characters
+// @param base_options the default options of the output "new_options".
-// prefixed by '\' in the values of the opts_map will be further
+// @param opts_map an option name to value map for specifying how "new_options"
-// converted back to the raw string before assigning to the associated
+//     should be set.
-// options.
+// @param new_options the resulting options based on "base_options" with the
+//     change specified in "opts_map".
+// @param input_strings_escaped when set to true, each escaped characters
+//     prefixed by '\' in the values of the opts_map will be further converted
+//     back to the raw string before assigning to the associated options.
+// @return Status::OK() on success.  Otherwise, a non-ok status indicating
+//     error will be returned, and "new_options" will be set to "base_options".
 Status GetDBOptionsFromMap(
     const DBOptions& base_options,
     const std::unordered_map<std::string, std::string>& opts_map,
     DBOptions* new_options, bool input_strings_escaped = 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".
+//
+// Below are the instructions of how to config some non-primitive-typed
+// options in BlockBasedTableOptions:
+//
+// * filter_policy:
+//   We currently only support the following FilterPolicy in the convenience
+//   functions:
+//   - BloomFilter: use "bloomfilter:[bits_per_key]:[use_block_based_builder]"
+//     to specify BloomFilter.  The above string is equivalent to calling
+//     NewBloomFilterPolicy(bits_per_key, use_block_based_builder).
+//     [Example]:
+//     - Pass {"filter_policy", "bloomfilter:4:true"} in
+//       GetBlockBasedTableOptionsFromMap to use a BloomFilter with 4-bits
+//       per key and use_block_based_builder enabled.
+//
+// * block_cache / block_cache_compressed:
+//   We currently only support LRU cache in the GetOptions API.  The LRU
+//   cache can be set by directly specifying its size.
+//   [Example]:
+//   - Passing {"block_cache", "1M"} in GetBlockBasedTableOptionsFromMap is
+//     equivalent to setting block_cache using NewLRUCache(1024 * 1024).
+//
+// @param table_options the default options of the output "new_table_options".
+// @param opts_map an option name to value map for specifying how
+//     "new_table_options" should be set.
+// @param new_table_options the resulting options based on "table_options"
+//     with the change specified in "opts_map".
+// @param input_strings_escaped when set to true, each escaped characters
+//     prefixed by '\' in the values of the opts_map will be further converted
+//     back to the raw string before assigning to the associated options.
+// @return Status::OK() on success.  Otherwise, a non-ok status indicating
+//     error will be returned, and "new_table_options" will be set to
+//     "table_options".
 Status GetBlockBasedTableOptionsFromMap(
     const BlockBasedTableOptions& table_options,
     const std::unordered_map<std::string, std::string>& opts_map,
     BlockBasedTableOptions* new_table_options,
     bool input_strings_escaped = 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".
+//
+// @param table_options the default options of the output "new_table_options".
+// @param opts_map an option name to value map for specifying how
+//     "new_table_options" should be set.
+// @param new_table_options the resulting options based on "table_options"
+//     with the change specified in "opts_map".
+// @param input_strings_escaped when set to true, each escaped characters
+//     prefixed by '\' in the values of the opts_map will be further converted
+//     back to the raw string before assigning to the associated options.
+// @return Status::OK() on success.  Otherwise, a non-ok status indicating
+//     error will be returned, and "new_table_options" will be set to
+//     "table_options".
 Status GetPlainTableOptionsFromMap(
     const PlainTableOptions& table_options,
     const std::unordered_map<std::string, std::string>& opts_map,

util/options_helper.cc

@@ -1148,6 +1148,8 @@ Status GetBlockBasedTableOptionsFromMap(
            iter->second.verification !=
                OptionVerificationType::kByNameAllowNull &&
            iter->second.verification != OptionVerificationType::kDeprecated)) {
+        // Restore "new_options" to the default "base_options".
+        *new_table_options = table_options;
         return Status::InvalidArgument("Can't parse BlockBasedTableOptions:",
                                        o.first + " " + error_message);
       }
@@ -1188,6 +1190,8 @@ Status GetPlainTableOptionsFromMap(
            iter->second.verification !=
                OptionVerificationType::kByNameAllowNull &&
            iter->second.verification != OptionVerificationType::kDeprecated)) {
+        // Restore "new_options" to the default "base_options".
+        *new_table_options = table_options;
         return Status::InvalidArgument("Can't parse PlainTableOptions:",
                                         o.first + " " + error_message);
       }
@@ -1312,6 +1316,8 @@ Status GetColumnFamilyOptionsFromMapInternal(
         // the backward compatibility in the old public API defined in
         // rocksdb/convenience.h
       } else {
+        // Restore "new_options" to the default "base_options".
+        *new_options = base_options;
         return s;
       }
     }
@@ -1326,6 +1332,7 @@ Status GetColumnFamilyOptionsFromString(
   std::unordered_map<std::string, std::string> opts_map;
   Status s = StringToMap(opts_str, &opts_map);
   if (!s.ok()) {
+    *new_options = base_options;
     return s;
   }
   return GetColumnFamilyOptionsFromMap(base_options, opts_map, new_options);
@@ -1364,6 +1371,8 @@ Status GetDBOptionsFromMapInternal(
         // the backward compatibility in the old public API defined in
         // rocksdb/convenience.h
       } else {
+        // Restore "new_options" to the default "base_options".
+        *new_options = base_options;
         return s;
       }
     }
@@ -1378,6 +1387,7 @@ Status GetDBOptionsFromString(
   std::unordered_map<std::string, std::string> opts_map;
   Status s = StringToMap(opts_str, &opts_map);
   if (!s.ok()) {
+    *new_options = base_options;
     return s;
   }
   return GetDBOptionsFromMap(base_options, opts_map, new_options);

util/options_test.cc

@@ -197,12 +197,16 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) {
   cf_options_map["write_buffer_size"] = "hello";
   ASSERT_NOK(GetColumnFamilyOptionsFromMap(
              base_cf_opt, cf_options_map, &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(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));
   cf_options_map["unknown_option"] = "1";
+
   ASSERT_NOK(GetColumnFamilyOptionsFromMap(
              base_cf_opt, cf_options_map, &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(base_cf_opt, new_cf_opt));
 
   DBOptions base_db_opt;
   DBOptions new_db_opt;
@@ -280,15 +284,22 @@ TEST_F(OptionsTest, GetColumnFamilyOptionsFromStringTest) {
   ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt,
              "write_buffer_size=13;max_write_buffer_number_=14;",
               &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(base_cf_opt, new_cf_opt));
+
   // Wrong key/value pair
   ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt,
              "write_buffer_size=13;max_write_buffer_number;", &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(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_OK(RocksDBOptionsParser::VerifyCFOptions(base_cf_opt, new_cf_opt));
+
   // Missing option name
   ASSERT_NOK(GetColumnFamilyOptionsFromString(base_cf_opt,
              "write_buffer_size=13; =100;", &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(base_cf_opt, new_cf_opt));
 
   const int64_t kilo = 1024UL;
   const int64_t mega = 1024 * kilo;
@@ -350,35 +361,47 @@ TEST_F(OptionsTest, GetColumnFamilyOptionsFromStringTest) {
              "block_based_table_factory={{{block_size=4;};"
              "arena_block_size=1024",
              &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(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_OK(RocksDBOptionsParser::VerifyCFOptions(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_OK(RocksDBOptionsParser::VerifyCFOptions(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_OK(RocksDBOptionsParser::VerifyCFOptions(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_OK(RocksDBOptionsParser::VerifyCFOptions(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_NOK(GetColumnFamilyOptionsFromString(base_cf_opt,
               "optimize_filters_for_hits=junk",
               &new_cf_opt));
+  ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(base_cf_opt, new_cf_opt));
 
   // Nested plain table options
   // Emtpy
@@ -440,27 +463,43 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) {
              "cache_index_and_filter_blocks=1;index_type=kBinarySearch;"
              "bad_option=1",
              &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 index type
   ASSERT_NOK(GetBlockBasedTableOptionsFromString(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_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_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);
+
   // unrecognized filter policy config
   ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt,
              "cache_index_and_filter_blocks=1;"
              "filter_policy=bloomfilter:4",
              &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);
 }
 #endif  // !ROCKSDB_LITE