From 9b58c73c7c48769ef6f2e3a1e17ab33b9e28d43d Mon Sep 17 00:00:00 2001 From: Lei Jin Date: Tue, 2 Sep 2014 14:42:23 -0700 Subject: [PATCH] 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 --- db/db_impl.cc | 17 +++++++++-------- db/simple_table_db_test.cc | 2 +- include/rocksdb/table.h | 2 +- table/adaptive_table_factory.h | 2 +- table/block_based_table_factory.h | 2 +- table/cuckoo_table_factory.h | 2 +- table/plain_table_factory.h | 2 +- 7 files changed, 15 insertions(+), 14 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index c22aa5809bc..7c65e9a61d3 100644 --- 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& 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 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& column_families, std::vector* 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 3a5809774de..e88485070e5 100644 --- a/db/simple_table_db_test.cc +++ b/db/simple_table_db_test.cc @@ -556,7 +556,7 @@ class SimpleTableFactory: public TableFactory { 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 3a47ed939b6..0f8b41074d7 100644 --- 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 571e0749895..f119d97b1cd 100644 --- 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 90282bf9d76..d7045346af0 100644 --- 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 06f657d223a..5799a7f23fe 100644 --- 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 31e20b016eb..d1cf0cae61a 100644 --- 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.");