From 6e56ab5702566a7afdb26e837f2997ac7671268e Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Wed, 5 Feb 2014 12:20:40 -0800 Subject: [PATCH] [CF] Add full_options_ to ColumnFamilyData Summary: Lots of code expects Options on construction/function call. My original idea was to split Options argument into ColumnFamilyOptions and DBOptions (the latter only if needed). However, this will require huge code changes very deep in the stack. The better idea is to have ColumnFamilyData hold both ColumnFamilyOptions and Options. ColumnFamilyData::Options would be constructed from DBOptions (same for each column family) and ColumnFamilyOptions (different for each column family) Now when we construct a class or call any method that requires Options, we can just push him ColumnFamilyData::Options and be sure that it's using column-family-specific settings. Test Plan: make check Reviewers: dhruba, haobo, kailiu, sdong CC: leveldb Differential Revision: https://reviews.facebook.net/D15927 --- db/column_family.cc | 11 ++++++----- db/column_family.h | 6 ++++-- db/repair.cc | 5 ++--- db/table_cache.cc | 22 +++++++++------------- db/table_cache.h | 6 ++---- 5 files changed, 23 insertions(+), 27 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index b38fd42ef..90cd36f6f 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -142,6 +142,7 @@ ColumnFamilyData::ColumnFamilyData(const std::string& dbname, uint32_t id, internal_filter_policy_(options.filter_policy), options_(SanitizeOptions(&internal_comparator_, &internal_filter_policy_, options)), + full_options_(DBOptions(*db_options), options_), mem_(nullptr), imm_(options.min_write_buffer_number_to_merge), super_version_(nullptr), @@ -150,12 +151,12 @@ ColumnFamilyData::ColumnFamilyData(const std::string& dbname, uint32_t id, prev_(nullptr), log_number_(0), need_slowdown_for_num_level0_files_(false) { - // if db_options is nullptr, then this is a dummy column family. - if (db_options != nullptr) { + // if dummy_versions is nullptr, then this is a dummy column family. + if (dummy_versions != nullptr) { internal_stats_.reset(new InternalStats(options.num_levels, db_options->env, db_options->statistics.get())); - table_cache_.reset(new TableCache(dbname, db_options, &options_, - storage_options, table_cache)); + table_cache_.reset( + new TableCache(dbname, &full_options_, storage_options, table_cache)); if (options_.compaction_style == kCompactionStyleUniversal) { compaction_picker_.reset(new UniversalCompactionPicker( &options_, &internal_comparator_, db_options->info_log.get())); @@ -241,7 +242,7 @@ ColumnFamilySet::ColumnFamilySet(const std::string& dbname, Cache* table_cache) : max_column_family_(0), dummy_cfd_(new ColumnFamilyData(dbname, 0, "", nullptr, nullptr, - ColumnFamilyOptions(), nullptr, + ColumnFamilyOptions(), db_options, storage_options_)), db_name_(dbname), db_options_(db_options), diff --git a/db/column_family.h b/db/column_family.h index 46b3a071a..81e5be68a 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -73,7 +73,8 @@ class ColumnFamilyData { void SetLogNumber(uint64_t log_number) { log_number_ = log_number; } uint64_t GetLogNumber() const { return log_number_; } - ColumnFamilyOptions* options() { return &options_; } + const ColumnFamilyOptions* options() const { return &options_; } + const Options* full_options() const { return &full_options_; } InternalStats* internal_stats(); MemTableList* imm() { return &imm_; } @@ -137,7 +138,8 @@ class ColumnFamilyData { const InternalKeyComparator internal_comparator_; const InternalFilterPolicy internal_filter_policy_; - ColumnFamilyOptions options_; + ColumnFamilyOptions const options_; + Options const full_options_; std::unique_ptr table_cache_; diff --git a/db/repair.cc b/db/repair.cc index 3b46b5854..fbf947615 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -55,15 +55,14 @@ class Repairer { icmp_(options.comparator), ipolicy_(options.filter_policy), options_(SanitizeOptions(dbname, &icmp_, &ipolicy_, options)), - cf_options_(ColumnFamilyOptions(options_)), raw_table_cache_( // TableCache can be small since we expect each table to be opened // once. NewLRUCache(10, options_.table_cache_numshardbits, options_.table_cache_remove_scan_count_limit)), next_file_number_(1) { - table_cache_ = new TableCache(dbname_, &options_, &cf_options_, - storage_options_, raw_table_cache_.get()); + table_cache_ = new TableCache(dbname_, &options_, storage_options_, + raw_table_cache_.get()); edit_ = new VersionEdit(); } diff --git a/db/table_cache.cc b/db/table_cache.cc index 33d720e94..adf94182d 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -35,13 +35,11 @@ static Slice GetSliceForFileNumber(uint64_t* file_number) { } // TODO(icanadi) Options -> DBOptions -TableCache::TableCache(const std::string& dbname, const Options* db_options, - const ColumnFamilyOptions* cf_options, +TableCache::TableCache(const std::string& dbname, const Options* options, const EnvOptions& storage_options, Cache* const cache) - : env_(db_options->env), + : env_(options->env), dbname_(dbname), - db_options_(db_options), - cf_options_(cf_options), + options_(options), storage_options_(storage_options), cache_(cache) {} @@ -66,21 +64,19 @@ Status TableCache::FindTable(const EnvOptions& toptions, unique_ptr file; unique_ptr table_reader; s = env_->NewRandomAccessFile(fname, &file, toptions); - RecordTick(db_options_->statistics.get(), NO_FILE_OPENS); + RecordTick(options_->statistics.get(), NO_FILE_OPENS); if (s.ok()) { - if (db_options_->advise_random_on_open) { + if (options_->advise_random_on_open) { file->Hint(RandomAccessFile::RANDOM); } - StopWatch sw(env_, db_options_->statistics.get(), TABLE_OPEN_IO_MICROS); - // TODO(icanadi) terrible hack. fix this - Options options(DBOptions(*db_options_), *cf_options_); - s = cf_options_->table_factory->GetTableReader( - options, toptions, std::move(file), file_size, &table_reader); + StopWatch sw(env_, options_->statistics.get(), TABLE_OPEN_IO_MICROS); + s = options_->table_factory->GetTableReader( + *options_, toptions, std::move(file), file_size, &table_reader); } if (!s.ok()) { assert(table_reader == nullptr); - RecordTick(db_options_->statistics.get(), NO_FILE_ERRORS); + RecordTick(options_->statistics.get(), NO_FILE_ERRORS); // We do not cache error results so that if the error is transient, // or somebody repairs the file, we recover automatically. } else { diff --git a/db/table_cache.h b/db/table_cache.h index 0a462acd9..9807aeb00 100644 --- a/db/table_cache.h +++ b/db/table_cache.h @@ -24,8 +24,7 @@ class Env; class TableCache { public: - TableCache(const std::string& dbname, const Options* db_options, - const ColumnFamilyOptions* cf_options, + TableCache(const std::string& dbname, const Options* options, const EnvOptions& storage_options, Cache* cache); ~TableCache(); @@ -67,8 +66,7 @@ class TableCache { private: Env* const env_; const std::string dbname_; - const Options* db_options_; - const ColumnFamilyOptions* cf_options_; + const Options* options_; const EnvOptions& storage_options_; Cache* const cache_;