DB::GetOptions() reflect dynamic changed options

Summary: DB::GetOptions() reflect dynamic changed options.

Test Plan: See the new unit test.

Reviewers: yhchiang, sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

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

HISTORY.md

@@ -1,4 +1,8 @@
 # Rocksdb Change Log
+## Unreleased
+### Public API Change
+* DB::GetOptions() reflect dynamic changed options (i.e. through DB::SetOptions()) and return copy of options instead of reference.
+
 ## 4.12.0 (9/12/2016)
 ### Public API Change
 * CancelAllBackgroundWork() flushes all memtables for databases containing writes that have bypassed the WAL (writes issued with WriteOptions::disableWAL=true) before shutting down background threads.

db/column_family.cc

@@ -482,6 +482,10 @@ void ColumnFamilyData::SetDropped() {
   column_family_set_->RemoveColumnFamily(this);
 }
 
+ColumnFamilyOptions ColumnFamilyData::GetLatestCFOptions() const {
+  return BuildColumnFamilyOptions(options_, mutable_cf_options_);
+}
+
 const double kSlowdownRatio = 1.2;
 
 namespace {

db/column_family.h

@@ -218,6 +218,12 @@ class ColumnFamilyData {
   const MutableCFOptions* GetLatestMutableCFOptions() const {
     return &mutable_cf_options_;
   }
+
+  // REQUIRES: DB mutex held
+  // Build ColumnFamiliesOptions with immutable options and latest mutable
+  // options.
+  ColumnFamilyOptions GetLatestCFOptions() const;
+
 #ifndef ROCKSDB_LITE
   // REQUIRES: DB mutex held
   Status SetOptions(

db/db_impl.cc

@@ -5110,9 +5110,10 @@ Env* DBImpl::GetEnv() const {
   return env_;
 }
 
-const Options& DBImpl::GetOptions(ColumnFamilyHandle* column_family) const {
+Options DBImpl::GetOptions(ColumnFamilyHandle* column_family) const {
+  InstrumentedMutexLock l(&mutex_);
   auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
-  return *cfh->cfd()->options();
+  return Options(db_options_, cfh->cfd()->GetLatestCFOptions());
 }
 
 const DBOptions& DBImpl::GetDBOptions() const { return db_options_; }
@@ -5993,8 +5994,7 @@ Status DBImpl::WriteOptionsFile() {
       continue;
     }
     cf_names.push_back(cfd->GetName());
-    cf_opts.push_back(BuildColumnFamilyOptions(
+    cf_opts.push_back(cfd->GetLatestCFOptions());
-        *cfd->options(), *cfd->GetLatestMutableCFOptions()));
   }
 
   // Unlock during expensive operations.  New writes cannot get here

db/db_impl.h

@@ -165,8 +165,7 @@ class DBImpl : public DB {
   virtual const std::string& GetName() const override;
   virtual Env* GetEnv() const override;
   using DB::GetOptions;
-  virtual const Options& GetOptions(
+  virtual Options GetOptions(ColumnFamilyHandle* column_family) const override;
-      ColumnFamilyHandle* column_family) const override;
   using DB::GetDBOptions;
   virtual const DBOptions& GetDBOptions() const override;
   using DB::Flush;
@@ -717,7 +716,7 @@ class DBImpl : public DB {
   //       same time.
   InstrumentedMutex options_files_mutex_;
   // State below is protected by mutex_
-  InstrumentedMutex mutex_;
+  mutable InstrumentedMutex mutex_;
 
   std::atomic<bool> shutting_down_;
   // This condition variable is signaled on these conditions:

db/db_options_test.cc

@@ -8,21 +8,71 @@
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 #include <limits>
 #include <string>
+#include <unordered_map>
 
+#include "db/column_family.h"
 #include "db/db_test_util.h"
 #include "port/stack_trace.h"
+#include "rocksdb/convenience.h"
+#include "util/options_helper.h"
+#include "util/random.h"
 #include "util/sync_point.h"
+#include "util/testutil.h"
 
 namespace rocksdb {
 
 class DBOptionsTest : public DBTestBase {
  public:
   DBOptionsTest() : DBTestBase("/db_options_test") {}
+
+#ifndef ROCKSDB_LITE
+  std::unordered_map<std::string, std::string> GetMutableCFOptionsMap(
+      const ColumnFamilyOptions& options) {
+    std::string options_str;
+    GetStringFromColumnFamilyOptions(&options_str, options);
+    std::unordered_map<std::string, std::string> options_map;
+    StringToMap(options_str, &options_map);
+    std::unordered_map<std::string, std::string> mutable_map;
+    for (const auto opt : cf_options_type_info) {
+      if (opt.second.is_mutable &&
+          opt.second.verification != OptionVerificationType::kDeprecated) {
+        mutable_map[opt.first] = options_map[opt.first];
+      }
+    }
+    return mutable_map;
+  }
+
+  std::unordered_map<std::string, std::string> GetRandomizedMutableCFOptionsMap(
+      Random* rnd) {
+    Options options;
+    test::RandomInitCFOptions(&options, rnd);
+    auto sanitized_options = SanitizeOptions(options, nullptr, options);
+    return GetMutableCFOptionsMap(sanitized_options);
+  }
+#endif  // ROCKSDB_LITE
 };
 
 // RocksDB lite don't support dynamic options.
 #ifndef ROCKSDB_LITE
 
+TEST_F(DBOptionsTest, GetLatestOptions) {
+  // GetOptions should be able to get latest option changed by SetOptions.
+  Options options;
+  options.create_if_missing = true;
+  Random rnd(228);
+  Reopen(options);
+  CreateColumnFamilies({"foo"}, options);
+  ReopenWithColumnFamilies({"default", "foo"}, options);
+  auto options_default = GetRandomizedMutableCFOptionsMap(&rnd);
+  auto options_foo = GetRandomizedMutableCFOptionsMap(&rnd);
+  ASSERT_OK(dbfull()->SetOptions(handles_[0], options_default));
+  ASSERT_OK(dbfull()->SetOptions(handles_[1], options_foo));
+  ASSERT_EQ(options_default,
+            GetMutableCFOptionsMap(dbfull()->GetOptions(handles_[0])));
+  ASSERT_EQ(options_foo,
+            GetMutableCFOptionsMap(dbfull()->GetOptions(handles_[1])));
+}
+
 TEST_F(DBOptionsTest, EnableAutoCompactionAndTriggerStall) {
   const std::string kValue(1024, 'v');
   for (int method_type = 0; method_type < 2; method_type++) {

db/db_test.cc

@@ -2806,8 +2806,7 @@ class ModelDB : public DB {
   virtual Env* GetEnv() const override { return nullptr; }
 
   using DB::GetOptions;
-  virtual const Options& GetOptions(
+  virtual Options GetOptions(ColumnFamilyHandle* column_family) const override {
-      ColumnFamilyHandle* column_family) const override {
     return options_;
   }
 

include/rocksdb/convenience.h

@@ -282,7 +282,7 @@ Status GetStringFromDBOptions(std::string* opts_str,
                               const std::string& delimiter = ";  ");
 
 Status GetStringFromColumnFamilyOptions(std::string* opts_str,
-                                        const ColumnFamilyOptions& db_options,
+                                        const ColumnFamilyOptions& cf_options,
                                         const std::string& delimiter = ";  ");
 
 Status GetStringFromCompressionType(std::string* compression_str,

include/rocksdb/db.h

@@ -696,9 +696,8 @@ class DB {
   // column family, the options provided when calling DB::Open() or
   // DB::CreateColumnFamily() will have been "sanitized" and transformed
   // in an implementation-defined manner.
-  virtual const Options& GetOptions(ColumnFamilyHandle* column_family)
+  virtual Options GetOptions(ColumnFamilyHandle* column_family) const = 0;
-      const = 0;
+  virtual Options GetOptions() const {
-  virtual const Options& GetOptions() const {
     return GetOptions(DefaultColumnFamily());
   }
 

include/rocksdb/utilities/stackable_db.h

@@ -219,8 +219,7 @@ class StackableDB : public DB {
   }
 
   using DB::GetOptions;
-  virtual const Options& GetOptions(ColumnFamilyHandle* column_family) const
+  virtual Options GetOptions(ColumnFamilyHandle* column_family) const override {
-      override {
     return db_->GetOptions(column_family);
   }
 

util/options_helper.cc

@@ -26,6 +26,65 @@
 
 namespace rocksdb {
 
+ColumnFamilyOptions BuildColumnFamilyOptions(
+    const ColumnFamilyOptions& options,
+    const MutableCFOptions& mutable_cf_options) {
+  ColumnFamilyOptions cf_opts(options);
+
+  // Memtable related options
+  cf_opts.write_buffer_size = mutable_cf_options.write_buffer_size;
+  cf_opts.max_write_buffer_number = mutable_cf_options.max_write_buffer_number;
+  cf_opts.arena_block_size = mutable_cf_options.arena_block_size;
+  cf_opts.memtable_prefix_bloom_size_ratio =
+      mutable_cf_options.memtable_prefix_bloom_size_ratio;
+  cf_opts.memtable_huge_page_size = mutable_cf_options.memtable_huge_page_size;
+  cf_opts.max_successive_merges = mutable_cf_options.max_successive_merges;
+  cf_opts.inplace_update_num_locks =
+      mutable_cf_options.inplace_update_num_locks;
+
+  // Compaction related options
+  cf_opts.disable_auto_compactions =
+      mutable_cf_options.disable_auto_compactions;
+  cf_opts.level0_file_num_compaction_trigger =
+      mutable_cf_options.level0_file_num_compaction_trigger;
+  cf_opts.level0_slowdown_writes_trigger =
+      mutable_cf_options.level0_slowdown_writes_trigger;
+  cf_opts.level0_stop_writes_trigger =
+      mutable_cf_options.level0_stop_writes_trigger;
+  cf_opts.max_compaction_bytes = mutable_cf_options.max_compaction_bytes;
+  cf_opts.target_file_size_base = mutable_cf_options.target_file_size_base;
+  cf_opts.target_file_size_multiplier =
+      mutable_cf_options.target_file_size_multiplier;
+  cf_opts.max_bytes_for_level_base =
+      mutable_cf_options.max_bytes_for_level_base;
+  cf_opts.max_bytes_for_level_multiplier =
+      mutable_cf_options.max_bytes_for_level_multiplier;
+
+  cf_opts.max_bytes_for_level_multiplier_additional.clear();
+  for (auto value :
+       mutable_cf_options.max_bytes_for_level_multiplier_additional) {
+    cf_opts.max_bytes_for_level_multiplier_additional.emplace_back(value);
+  }
+
+  cf_opts.verify_checksums_in_compaction =
+      mutable_cf_options.verify_checksums_in_compaction;
+
+  // Misc options
+  cf_opts.max_sequential_skip_in_iterations =
+      mutable_cf_options.max_sequential_skip_in_iterations;
+  cf_opts.paranoid_file_checks = mutable_cf_options.paranoid_file_checks;
+  cf_opts.report_bg_io_stats = mutable_cf_options.report_bg_io_stats;
+  cf_opts.compression = mutable_cf_options.compression;
+  cf_opts.min_partial_merge_operands =
+      mutable_cf_options.min_partial_merge_operands;
+
+  cf_opts.table_factory = options.table_factory;
+  // TODO(yhchiang): find some way to handle the following derived options
+  // * max_file_size
+
+  return cf_opts;
+}
+
 #ifndef ROCKSDB_LITE
 bool isSpecialChar(const char c) {
   if (c == '\\' || c == '#' || c == ':' || c == '\r' || c == '\n') {
@@ -1367,60 +1426,6 @@ Status GetTableFactoryFromMap(
   return Status::OK();
 }
 
-ColumnFamilyOptions BuildColumnFamilyOptions(
-    const Options& options, const MutableCFOptions& mutable_cf_options) {
-  ColumnFamilyOptions cf_opts(options);
-
-  // Memtable related options
-  cf_opts.write_buffer_size = mutable_cf_options.write_buffer_size;
-  cf_opts.max_write_buffer_number = mutable_cf_options.max_write_buffer_number;
-  cf_opts.arena_block_size = mutable_cf_options.arena_block_size;
-  cf_opts.memtable_prefix_bloom_size_ratio =
-      mutable_cf_options.memtable_prefix_bloom_size_ratio;
-  cf_opts.memtable_huge_page_size = mutable_cf_options.memtable_huge_page_size;
-  cf_opts.max_successive_merges = mutable_cf_options.max_successive_merges;
-  cf_opts.inplace_update_num_locks =
-      mutable_cf_options.inplace_update_num_locks;
-
-  // Compaction related options
-  cf_opts.disable_auto_compactions =
-      mutable_cf_options.disable_auto_compactions;
-  cf_opts.level0_file_num_compaction_trigger =
-      mutable_cf_options.level0_file_num_compaction_trigger;
-  cf_opts.level0_slowdown_writes_trigger =
-      mutable_cf_options.level0_slowdown_writes_trigger;
-  cf_opts.level0_stop_writes_trigger =
-      mutable_cf_options.level0_stop_writes_trigger;
-  cf_opts.max_compaction_bytes = mutable_cf_options.max_compaction_bytes;
-  cf_opts.target_file_size_base = mutable_cf_options.target_file_size_base;
-  cf_opts.target_file_size_multiplier =
-      mutable_cf_options.target_file_size_multiplier;
-  cf_opts.max_bytes_for_level_base =
-      mutable_cf_options.max_bytes_for_level_base;
-  cf_opts.max_bytes_for_level_multiplier =
-      mutable_cf_options.max_bytes_for_level_multiplier;
-
-  cf_opts.max_bytes_for_level_multiplier_additional.clear();
-  for (auto value :
-       mutable_cf_options.max_bytes_for_level_multiplier_additional) {
-    cf_opts.max_bytes_for_level_multiplier_additional.emplace_back(value);
-  }
-
-  cf_opts.verify_checksums_in_compaction =
-      mutable_cf_options.verify_checksums_in_compaction;
-
-  // Misc options
-  cf_opts.max_sequential_skip_in_iterations =
-      mutable_cf_options.max_sequential_skip_in_iterations;
-  cf_opts.paranoid_file_checks = mutable_cf_options.paranoid_file_checks;
-  cf_opts.report_bg_io_stats = mutable_cf_options.report_bg_io_stats;
-
-  cf_opts.table_factory = options.table_factory;
-  // TODO(yhchiang): find some way to handle the following derived options
-  // * max_file_size
-
-  return cf_opts;
-}
-
 #endif  // !ROCKSDB_LITE
+
 }  // namespace rocksdb

util/options_helper.h

@@ -14,9 +14,14 @@
 #include "rocksdb/table.h"
 #include "util/cf_options.h"
 
-#ifndef ROCKSDB_LITE
 namespace rocksdb {
 
+ColumnFamilyOptions BuildColumnFamilyOptions(
+    const ColumnFamilyOptions& ioptions,
+    const MutableCFOptions& mutable_cf_options);
+
+#ifndef ROCKSDB_LITE
+
 // Returns true if the input char "c" is considered as a special character
 // that will be escaped when EscapeOptionString() is called.
 //
@@ -68,9 +73,6 @@ Status GetTableFactoryFromMap(
 Status GetStringFromTableFactory(std::string* opts_str, const TableFactory* tf,
                                  const std::string& delimiter = ";  ");
 
-ColumnFamilyOptions BuildColumnFamilyOptions(
-    const Options& options, const MutableCFOptions& mutable_cf_options);
-
 enum class OptionType {
   kBoolean,
   kInt,
@@ -672,6 +674,7 @@ static std::unordered_map<std::string, InfoLogLevel> info_log_level_string_map =
      {"FATAL_LEVEL", InfoLogLevel::FATAL_LEVEL},
      {"HEADER_LEVEL", InfoLogLevel::HEADER_LEVEL}};
 
+#endif  // !ROCKSDB_LITE
+
 }  // namespace rocksdb
 
-#endif  // !ROCKSDB_LITE

utilities/backupable/backupable_db_test.cc

@@ -57,8 +57,7 @@ class DummyDB : public StackableDB {
   }
 
   using DB::GetOptions;
-  virtual const Options& GetOptions(ColumnFamilyHandle* column_family) const
+  virtual Options GetOptions(ColumnFamilyHandle* column_family) const override {
-      override {
     return options_;
   }