call SanitizeDBOptionsByCFOptions() in the right place

Summary: It only covers Open() with default column family right now

Test Plan: make release

Reviewers: igor, yhchiang, sdong

Reviewed By: sdong

Subscribers: leveldb

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

diff --git a/db/db_impl.cc b/db/db_impl.cc
index c22aa5809bc1937209634a5ba9f27359578e506d..7c65e9a61d37c9c933069c50e3ca605dd13055c9 100644 (file)
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -290,8 +290,10 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) {
   return result;
 }
 
+namespace {
+
 Status SanitizeDBOptionsByCFOptions(
-    DBOptions* db_opts,
+    const DBOptions* db_opts,
     const std::vector<ColumnFamilyDescriptor>& column_families) {
   Status s;
   for (auto cf : column_families) {
@@ -303,7 +305,6 @@ Status SanitizeDBOptionsByCFOptions(
   return Status::OK();
 }
 
-namespace {
 CompressionType GetCompressionFlush(const Options& options) {
   // Compressing memtable flushes might not help unless the sequential load
   // optimization is used for leveled compaction. Otherwise the CPU and
@@ -4802,11 +4803,7 @@ Status DB::Open(const Options& options, const std::string& dbname, DB** dbptr) {
   column_families.push_back(
       ColumnFamilyDescriptor(kDefaultColumnFamilyName, cf_options));
   std::vector<ColumnFamilyHandle*> handles;
-  Status s = SanitizeDBOptionsByCFOptions(&db_options, column_families);
-  if (!s.ok()) {
-    return s;
-  }
-  s = DB::Open(db_options, dbname, column_families, &handles, dbptr);
+  Status s = DB::Open(db_options, dbname, column_families, &handles, dbptr);
   if (s.ok()) {
     assert(handles.size() == 1);
     // i can delete the handle since DBImpl is always holding a reference to
@@ -4819,6 +4816,10 @@ Status DB::Open(const Options& options, const std::string& dbname, DB** dbptr) {
 Status DB::Open(const DBOptions& db_options, const std::string& dbname,
                 const std::vector<ColumnFamilyDescriptor>& column_families,
                 std::vector<ColumnFamilyHandle*>* handles, DB** dbptr) {
+  Status s = SanitizeDBOptionsByCFOptions(&db_options, column_families);
+  if (!s.ok()) {
+    return s;
+  }
   if (db_options.db_paths.size() > 1) {
     for (auto& cfd : column_families) {
       if (cfd.options.compaction_style != kCompactionStyleUniversal) {
@@ -4844,7 +4845,7 @@ Status DB::Open(const DBOptions& db_options, const std::string& dbname,
   }
 
   DBImpl* impl = new DBImpl(db_options, dbname);
-  Status s = impl->env_->CreateDirIfMissing(impl->options_.wal_dir);
+  s = impl->env_->CreateDirIfMissing(impl->options_.wal_dir);
   if (s.ok()) {
     for (auto db_path : impl->options_.db_paths) {
       s = impl->env_->CreateDirIfMissing(db_path.path);

diff --git a/db/simple_table_db_test.cc b/db/simple_table_db_test.cc
index 3a5809774de232a3a5a9c392f3873f639c4cc297..e88485070e5ad42a72dfa5e31afb6c1651d48b74 100644 (file)
--- a/db/simple_table_db_test.cc
+++ b/db/simple_table_db_test.cc
@@ -556,7 +556,7 @@ public:
                                 WritableFile* file,
                                 CompressionType compression_type) const;
 
-  virtual Status SanitizeDBOptions(DBOptions* db_opts) const override {
+  virtual Status SanitizeDBOptions(const DBOptions* db_opts) const override {
     return Status::OK();
   }
 

diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h
index 3a47ed939b645c9d4c4dd86b012cc164deafc010..0f8b41074d7cc7f60dab5d8330daa4b02486e6b2 100644 (file)
--- a/include/rocksdb/table.h
+++ b/include/rocksdb/table.h
@@ -331,7 +331,7 @@ class TableFactory {
   //
   // If the function cannot find a way to sanitize the input DB Options,
   // a non-ok Status will be returned.
-  virtual Status SanitizeDBOptions(DBOptions* db_opts) const = 0;
+  virtual Status SanitizeDBOptions(const DBOptions* db_opts) const = 0;
 
   // Return a string that contains printable format of table configurations.
   // RocksDB prints configurations at DB Open().

diff --git a/table/adaptive_table_factory.h b/table/adaptive_table_factory.h
index 571e0749895c5da58c405b44804982ebe88da416..f119d97b1cd3c44142ae4a43c46c1129560f3de3 100644 (file)
--- a/table/adaptive_table_factory.h
+++ b/table/adaptive_table_factory.h
@@ -43,7 +43,7 @@ class AdaptiveTableFactory : public TableFactory {
       override;
 
   // Sanitizes the specified DB Options.
-  Status SanitizeDBOptions(DBOptions* db_opts) const override {
+  Status SanitizeDBOptions(const DBOptions* db_opts) const override {
     if (db_opts->allow_mmap_reads == false) {
       return Status::NotSupported(
           "AdaptiveTable with allow_mmap_reads == false is not supported.");

diff --git a/table/block_based_table_factory.h b/table/block_based_table_factory.h
index 90282bf9d7627880cabdce55c6fa1c522b586e87..d7045346af09781789708da9404c1008f7220743 100644 (file)
--- a/table/block_based_table_factory.h
+++ b/table/block_based_table_factory.h
@@ -45,7 +45,7 @@ class BlockBasedTableFactory : public TableFactory {
       WritableFile* file, CompressionType compression_type) const override;
 
   // Sanitizes the specified DB Options.
-  Status SanitizeDBOptions(DBOptions* db_opts) const override {
+  Status SanitizeDBOptions(const DBOptions* db_opts) const override {
     return Status::OK();
   }
 

diff --git a/table/cuckoo_table_factory.h b/table/cuckoo_table_factory.h
index 06f657d223afabf8ea690ce077f827bcec5f31d6..5799a7f23fe52983f3ebd708fa7e6c3bb0614cff 100644 (file)
--- a/table/cuckoo_table_factory.h
+++ b/table/cuckoo_table_factory.h
@@ -55,7 +55,7 @@ class CuckooTableFactory : public TableFactory {
       CompressionType compression_type) const override;
 
   // Sanitizes the specified DB Options.
-  Status SanitizeDBOptions(DBOptions* db_opts) const override {
+  Status SanitizeDBOptions(const DBOptions* db_opts) const override {
     return Status::OK();
   }
 

diff --git a/table/plain_table_factory.h b/table/plain_table_factory.h
index 31e20b016eb431997ac0b9995d4b648409085541..d1cf0cae61a129141d77d4117440381b1626730a 100644 (file)
--- a/table/plain_table_factory.h
+++ b/table/plain_table_factory.h
@@ -169,7 +169,7 @@ class PlainTableFactory : public TableFactory {
   static const char kValueTypeSeqId0 = 0xFF;
 
   // Sanitizes the specified DB Options.
-  Status SanitizeDBOptions(DBOptions* db_opts) const override {
+  Status SanitizeDBOptions(const DBOptions* db_opts) const override {
     if (db_opts->allow_mmap_reads == false) {
       return Status::NotSupported(
           "PlainTable with allow_mmap_reads == false is not supported.");