Skip to content

Commit

Permalink
call SanitizeDBOptionsByCFOptions() in the right place
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Lei Jin committed Sep 2, 2014
1 parent a84234a commit 9b58c73
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 14 deletions.
17 changes: 9 additions & 8 deletions db/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion db/simple_table_db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion include/rocksdb/table.h
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
2 changes: 1 addition & 1 deletion table/adaptive_table_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
2 changes: 1 addition & 1 deletion table/block_based_table_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion table/cuckoo_table_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion table/plain_table_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down

0 comments on commit 9b58c73

Please sign in to comment.