From 8d3f8f9696664378aff7f53f739ee6904ef0bbba Mon Sep 17 00:00:00 2001 From: Lei Jin Date: Tue, 18 Nov 2014 10:20:10 -0800 Subject: [PATCH] remove all remaining references to cfd->options() Summary: The very last reference happens in DBImpl::GetOptions() I built with both DBImpl::GetOptions() and ColumnFamilyData::options() commented out Test Plan: make all check Reviewers: sdong, yhchiang, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D29073 --- db/column_family.cc | 5 ----- db/column_family.h | 9 +++------ db/db_impl_debug.cc | 4 ++-- db/forward_iterator.cc | 2 +- db/internal_stats.cc | 4 ++-- db/repair.cc | 2 +- db/version_set.cc | 2 +- db/write_batch_internal.h | 11 ++--------- db/write_batch_test.cc | 2 +- include/rocksdb/options.h | 3 +++ java/rocksjni/write_batch.cc | 2 +- table/table_test.cc | 2 +- util/mutable_cf_options.cc | 2 ++ util/mutable_cf_options.h | 3 +++ util/options_helper.cc | 5 ++--- 15 files changed, 25 insertions(+), 33 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 5261acc8c..8456ed9ca 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -752,11 +752,6 @@ MemTable* ColumnFamilyMemTablesImpl::GetMemTable() const { return current_->mem(); } -const Options* ColumnFamilyMemTablesImpl::GetOptions() const { - assert(current_ != nullptr); - return current_->options(); -} - ColumnFamilyHandle* ColumnFamilyMemTablesImpl::GetColumnFamilyHandle() { assert(current_ != nullptr); return &handle_; diff --git a/db/column_family.h b/db/column_family.h index f24105fbe..c6d49e71b 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -172,9 +172,10 @@ class ColumnFamilyData { void SetLogNumber(uint64_t log_number) { log_number_ = log_number; } uint64_t GetLogNumber() const { return log_number_; } - // thread-safe - // To be deprecated! Please don't not use this function anymore! + // !!! To be deprecated! Please don't not use this function anymore! const Options* options() const { return &options_; } + + // thread-safe const EnvOptions* soptions() const; const ImmutableCFOptions* ioptions() const { return &ioptions_; } // REQUIRES: DB mutex held @@ -444,10 +445,6 @@ class ColumnFamilyMemTablesImpl : public ColumnFamilyMemTables { // REQUIRES: Seek() called first virtual MemTable* GetMemTable() const override; - // Returns options for selected column family - // REQUIRES: Seek() called first - virtual const Options* GetOptions() const override; - // Returns column family handle for the selected column family virtual ColumnFamilyHandle* GetColumnFamilyHandle() override; diff --git a/db/db_impl_debug.cc b/db/db_impl_debug.cc index ea8f5e13b..65eaff6b3 100644 --- a/db/db_impl_debug.cc +++ b/db/db_impl_debug.cc @@ -81,8 +81,8 @@ Status DBImpl::TEST_CompactRange(int level, const Slice* begin, cfd = cfh->cfd(); } int output_level = - (cfd->options()->compaction_style == kCompactionStyleUniversal || - cfd->options()->compaction_style == kCompactionStyleFIFO) + (cfd->ioptions()->compaction_style == kCompactionStyleUniversal || + cfd->ioptions()->compaction_style == kCompactionStyleFIFO) ? level : level + 1; return RunManualCompaction(cfd, level, output_level, 0, begin, end); diff --git a/db/forward_iterator.cc b/db/forward_iterator.cc index 7fd625a00..93af3c2d4 100644 --- a/db/forward_iterator.cc +++ b/db/forward_iterator.cc @@ -119,7 +119,7 @@ ForwardIterator::ForwardIterator(DBImpl* db, const ReadOptions& read_options, : db_(db), read_options_(read_options), cfd_(cfd), - prefix_extractor_(cfd->options()->prefix_extractor.get()), + prefix_extractor_(cfd->ioptions()->prefix_extractor), user_comparator_(cfd->user_comparator()), immutable_min_heap_(MinIterComparator(&cfd_->internal_comparator())), sv_(current_sv), diff --git a/db/internal_stats.cc b/db/internal_stats.cc index 1afe31520..33842fed8 100644 --- a/db/internal_stats.cc +++ b/db/internal_stats.cc @@ -361,8 +361,8 @@ void InternalStats::DumpCFStats(std::string* value) { const VersionStorageInfo* vstorage = cfd_->current()->storage_info(); int num_levels_to_check = - (cfd_->options()->compaction_style != kCompactionStyleUniversal && - cfd_->options()->compaction_style != kCompactionStyleFIFO) + (cfd_->ioptions()->compaction_style != kCompactionStyleUniversal && + cfd_->ioptions()->compaction_style != kCompactionStyleFIFO) ? vstorage->num_levels() - 1 : 1; diff --git a/db/repair.cc b/db/repair.cc index f23e757b0..8fa312638 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -222,7 +222,7 @@ class Repairer { WriteBatch batch; MemTable* mem = new MemTable(icmp_, ioptions_, MutableCFOptions(options_, ioptions_)); - auto cf_mems_default = new ColumnFamilyMemTablesDefault(mem, &options_); + auto cf_mems_default = new ColumnFamilyMemTablesDefault(mem); mem->Ref(); int counter = 0; while (reader.ReadRecord(&record, &scratch)) { diff --git a/db/version_set.cc b/db/version_set.cc index 97215ce0c..db8808687 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -2639,7 +2639,7 @@ Iterator* VersionSet::MakeInputIterator(Compaction* c) { auto cfd = c->column_family_data(); ReadOptions read_options; read_options.verify_checksums = - cfd->options()->verify_checksums_in_compaction; + c->mutable_cf_options()->verify_checksums_in_compaction; read_options.fill_cache = false; // Level-0 files have to be merged together. For other levels, diff --git a/db/write_batch_internal.h b/db/write_batch_internal.h index 568cd70d8..793c0d40f 100644 --- a/db/write_batch_internal.h +++ b/db/write_batch_internal.h @@ -26,15 +26,14 @@ class ColumnFamilyMemTables { // been processed) virtual uint64_t GetLogNumber() const = 0; virtual MemTable* GetMemTable() const = 0; - virtual const Options* GetOptions() const = 0; virtual ColumnFamilyHandle* GetColumnFamilyHandle() = 0; virtual void CheckMemtableFull() = 0; }; class ColumnFamilyMemTablesDefault : public ColumnFamilyMemTables { public: - ColumnFamilyMemTablesDefault(MemTable* mem, const Options* options) - : ok_(false), mem_(mem), options_(options) {} + explicit ColumnFamilyMemTablesDefault(MemTable* mem) + : ok_(false), mem_(mem) {} bool Seek(uint32_t column_family_id) override { ok_ = (column_family_id == 0); @@ -48,11 +47,6 @@ class ColumnFamilyMemTablesDefault : public ColumnFamilyMemTables { return mem_; } - const Options* GetOptions() const override { - assert(ok_); - return options_; - } - ColumnFamilyHandle* GetColumnFamilyHandle() override { return nullptr; } void CheckMemtableFull() override {} @@ -60,7 +54,6 @@ class ColumnFamilyMemTablesDefault : public ColumnFamilyMemTables { private: bool ok_; MemTable* mem_; - const Options* const options_; }; // WriteBatchInternal provides static methods for manipulating a diff --git a/db/write_batch_test.cc b/db/write_batch_test.cc index d24b2e068..7f180d9e6 100644 --- a/db/write_batch_test.cc +++ b/db/write_batch_test.cc @@ -32,7 +32,7 @@ static std::string PrintContents(WriteBatch* b) { MutableCFOptions(options, ioptions)); mem->Ref(); std::string state; - ColumnFamilyMemTablesDefault cf_mems_default(mem, &options); + ColumnFamilyMemTablesDefault cf_mems_default(mem); Status s = WriteBatchInternal::InsertInto(b, &cf_mems_default); int count = 0; Arena arena; diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index 102143301..dd05aa9de 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -421,7 +421,10 @@ struct ColumnFamilyOptions { // If true, compaction will verify checksum on every read that happens // as part of compaction + // // Default: true + // + // Dynamically changeable through SetOptions() API bool verify_checksums_in_compaction; // The options needed to support Universal Style compactions diff --git a/java/rocksjni/write_batch.cc b/java/rocksjni/write_batch.cc index d243c87a0..8adcfdc0f 100644 --- a/java/rocksjni/write_batch.cc +++ b/java/rocksjni/write_batch.cc @@ -344,7 +344,7 @@ jbyteArray Java_org_rocksdb_test_WriteBatchTest_getContents( rocksdb::ImmutableCFOptions(options))); mem->Ref(); std::string state; - rocksdb::ColumnFamilyMemTablesDefault cf_mems_default(mem, &options); + rocksdb::ColumnFamilyMemTablesDefault cf_mems_default(mem); rocksdb::Status s = rocksdb::WriteBatchInternal::InsertInto(b, &cf_mems_default); int count = 0; diff --git a/table/table_test.cc b/table/table_test.cc index facf0926e..a02846ccc 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -1879,7 +1879,7 @@ TEST(MemTableTest, Simple) { batch.Put(std::string("k2"), std::string("v2")); batch.Put(std::string("k3"), std::string("v3")); batch.Put(std::string("largekey"), std::string("vlarge")); - ColumnFamilyMemTablesDefault cf_mems_default(memtable, &options); + ColumnFamilyMemTablesDefault cf_mems_default(memtable); ASSERT_TRUE(WriteBatchInternal::InsertInto(&batch, &cf_mems_default).ok()); Arena arena; diff --git a/util/mutable_cf_options.cc b/util/mutable_cf_options.cc index c5f4c60b3..4ec2a4138 100644 --- a/util/mutable_cf_options.cc +++ b/util/mutable_cf_options.cc @@ -128,6 +128,8 @@ void MutableCFOptions::Dump(Logger* log) const { Log(log, "max_bytes_for_level_multiplier_additional: %s", result.c_str()); Log(log, " max_mem_compaction_level: %d", max_mem_compaction_level); + Log(log, " verify_checksums_in_compaction: %d", + verify_checksums_in_compaction); Log(log, " max_sequential_skip_in_iterations: %" PRIu64, max_sequential_skip_in_iterations); } diff --git a/util/mutable_cf_options.h b/util/mutable_cf_options.h index 40938655b..9f876ace0 100644 --- a/util/mutable_cf_options.h +++ b/util/mutable_cf_options.h @@ -40,6 +40,7 @@ struct MutableCFOptions { max_bytes_for_level_multiplier_additional( options.max_bytes_for_level_multiplier_additional), max_mem_compaction_level(options.max_mem_compaction_level), + verify_checksums_in_compaction(options.verify_checksums_in_compaction), max_sequential_skip_in_iterations( options.max_sequential_skip_in_iterations) { @@ -69,6 +70,7 @@ struct MutableCFOptions { max_bytes_for_level_base(0), max_bytes_for_level_multiplier(0), max_mem_compaction_level(0), + verify_checksums_in_compaction(false), max_sequential_skip_in_iterations(0) {} @@ -114,6 +116,7 @@ struct MutableCFOptions { int max_bytes_for_level_multiplier; std::vector max_bytes_for_level_multiplier_additional; int max_mem_compaction_level; + bool verify_checksums_in_compaction; // Misc options uint64_t max_sequential_skip_in_iterations; diff --git a/util/options_helper.cc b/util/options_helper.cc index 111215282..bea7f1a9d 100644 --- a/util/options_helper.cc +++ b/util/options_helper.cc @@ -185,6 +185,8 @@ bool ParseCompactionOptions(const std::string& name, const std::string& value, } } else if (name == "max_mem_compaction_level") { new_options->max_mem_compaction_level = ParseInt(value); + } else if (name == "verify_checksums_in_compaction") { + new_options->verify_checksums_in_compaction = ParseBoolean(name, value); } else { return false; } @@ -330,9 +332,6 @@ bool GetColumnFamilyOptionsFromMap( ParseBoolean(o.first, o.second); } else if (o.first == "compaction_style") { new_options->compaction_style = ParseCompactionStyle(o.second); - } else if (o.first == "verify_checksums_in_compaction") { - new_options->verify_checksums_in_compaction = - ParseBoolean(o.first, o.second); } else if (o.first == "compaction_options_universal") { // TODO(ljin): add support throw o.first;