From 2ad68b971aa60197abb6b56cdc5adc89e4642ccd Mon Sep 17 00:00:00 2001 From: Islam AbdelRahman Date: Fri, 7 Oct 2016 17:21:45 -0700 Subject: [PATCH] Support running consistency checks in release mode Summary: We always run consistency checks when compiling in debug mode allow users to set Options::force_consistency_checks to true to be able to run such checks even when compiling in release mode Test Plan: make check -j64 make release Reviewers: lightmark, sdong, yiwu Reviewed By: yiwu Subscribers: hermanlee4, andrewkr, yoshinorim, jkedgar, dhruba Differential Revision: https://reviews.facebook.net/D64701 --- db/compaction_picker_test.cc | 4 +-- db/version_builder.cc | 50 +++++++++++++++++++++++++---------- db/version_builder_test.cc | 12 ++++----- db/version_set.cc | 24 ++++++++++------- db/version_set.h | 9 ++++++- db/version_set_test.cc | 2 +- include/rocksdb/options.h | 6 +++++ util/cf_options.cc | 1 + util/cf_options.h | 2 ++ util/options.cc | 4 +++ util/options_helper.h | 3 +++ util/options_settable_test.cc | 1 + util/testutil.cc | 1 + 13 files changed, 85 insertions(+), 34 deletions(-) diff --git a/db/compaction_picker_test.cc b/db/compaction_picker_test.cc index e23c8e87d..ccda2f486 100644 --- a/db/compaction_picker_test.cc +++ b/db/compaction_picker_test.cc @@ -66,8 +66,8 @@ class CompactionPickerTest : public testing::Test { void NewVersionStorage(int num_levels, CompactionStyle style) { DeleteVersionStorage(); options_.num_levels = num_levels; - vstorage_.reset(new VersionStorageInfo( - &icmp_, ucmp_, options_.num_levels, style, nullptr)); + vstorage_.reset(new VersionStorageInfo(&icmp_, ucmp_, options_.num_levels, + style, nullptr, false)); vstorage_->CalculateBaseBytes(ioptions_, mutable_cf_options_); } diff --git a/db/version_builder.cc b/db/version_builder.cc index 2837686be..fac3c8ab2 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -127,7 +127,13 @@ class VersionBuilder::Rep { } void CheckConsistency(VersionStorageInfo* vstorage) { -#ifndef NDEBUG +#ifdef NDEBUG + if (!vstorage->force_consistency_checks()) { + // Dont run consistency checks in release mode except if + // explicitly asked to + return; + } +#endif // make sure the files are sorted correctly for (int level = 0; level < vstorage->num_levels(); level++) { auto& level_files = vstorage->LevelFiles(level); @@ -135,31 +141,48 @@ class VersionBuilder::Rep { auto f1 = level_files[i - 1]; auto f2 = level_files[i]; if (level == 0) { - assert(level_zero_cmp_(f1, f2)); - assert(f1->largest_seqno > f2->largest_seqno || - // We can have multiple files with seqno = 0 as a result of - // using DB::AddFile() - (f1->largest_seqno == 0 && f2->largest_seqno == 0)); + if (!level_zero_cmp_(f1, f2)) { + fprintf(stderr, "L0 files are not sorted properly"); + abort(); + } + + if (!(f1->largest_seqno > f2->largest_seqno || + // We can have multiple files with seqno = 0 as a result of + // using DB::AddFile() + (f1->largest_seqno == 0 && f2->largest_seqno == 0))) { + fprintf(stderr, + "L0 files seqno missmatch %" PRIu64 " vs. %" PRIu64 "\n", + f1->largest_seqno, f2->largest_seqno); + abort(); + } } else { - assert(level_nonzero_cmp_(f1, f2)); + if (!level_nonzero_cmp_(f1, f2)) { + fprintf(stderr, "L%d files are not sorted properly", level); + abort(); + } // Make sure there is no overlap in levels > 0 if (vstorage->InternalComparator()->Compare(f1->largest, f2->smallest) >= 0) { - fprintf(stderr, "overlapping ranges in same level %s vs. %s\n", - (f1->largest).DebugString().c_str(), - (f2->smallest).DebugString().c_str()); + fprintf(stderr, "L%d have overlapping ranges %s vs. %s\n", level, + (f1->largest).DebugString(true).c_str(), + (f2->smallest).DebugString(true).c_str()); abort(); } } } } -#endif } void CheckConsistencyForDeletes(VersionEdit* edit, uint64_t number, int level) { -#ifndef NDEBUG +#ifdef NDEBUG + if (!base_vstorage_->force_consistency_checks()) { + // Dont run consistency checks in release mode except if + // explicitly asked to + return; + } +#endif // a file to be deleted better exist in the previous version bool found = false; for (int l = 0; !found && l < base_vstorage_->num_levels(); l++) { @@ -195,9 +218,8 @@ class VersionBuilder::Rep { } if (!found) { fprintf(stderr, "not found %" PRIu64 "\n", number); + abort(); } - assert(found); -#endif } // Apply all of the edits in *edit to the current state. diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 501e42d4e..389c85a1e 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -31,7 +31,7 @@ class VersionBuilderTest : public testing::Test { ioptions_(options_), mutable_cf_options_(options_), vstorage_(&icmp_, ucmp_, options_.num_levels, kCompactionStyleLevel, - nullptr), + nullptr, false), file_num_(1) { mutable_cf_options_.RefreshDerivedOptions(ioptions_); size_being_compacted_.resize(options_.num_levels); @@ -123,7 +123,7 @@ TEST_F(VersionBuilderTest, ApplyAndSaveTo) { VersionBuilder version_builder(env_options, nullptr, &vstorage_); VersionStorageInfo new_vstorage(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleLevel, nullptr); + kCompactionStyleLevel, nullptr, false); version_builder.Apply(&version_edit); version_builder.SaveTo(&new_vstorage); @@ -158,7 +158,7 @@ TEST_F(VersionBuilderTest, ApplyAndSaveToDynamic) { VersionBuilder version_builder(env_options, nullptr, &vstorage_); VersionStorageInfo new_vstorage(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleLevel, nullptr); + kCompactionStyleLevel, nullptr, false); version_builder.Apply(&version_edit); version_builder.SaveTo(&new_vstorage); @@ -198,7 +198,7 @@ TEST_F(VersionBuilderTest, ApplyAndSaveToDynamic2) { VersionBuilder version_builder(env_options, nullptr, &vstorage_); VersionStorageInfo new_vstorage(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleLevel, nullptr); + kCompactionStyleLevel, nullptr, false); version_builder.Apply(&version_edit); version_builder.SaveTo(&new_vstorage); @@ -229,7 +229,7 @@ TEST_F(VersionBuilderTest, ApplyMultipleAndSaveTo) { VersionBuilder version_builder(env_options, nullptr, &vstorage_); VersionStorageInfo new_vstorage(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleLevel, nullptr); + kCompactionStyleLevel, nullptr, false); version_builder.Apply(&version_edit); version_builder.SaveTo(&new_vstorage); @@ -244,7 +244,7 @@ TEST_F(VersionBuilderTest, ApplyDeleteAndSaveTo) { EnvOptions env_options; VersionBuilder version_builder(env_options, nullptr, &vstorage_); VersionStorageInfo new_vstorage(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleLevel, nullptr); + kCompactionStyleLevel, nullptr, false); VersionEdit version_edit; version_edit.AddFile(2, 666, 0, 100U, GetInternalKey("301"), diff --git a/db/version_set.cc b/db/version_set.cc index bda5216f2..b58eab415 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -845,7 +845,8 @@ void Version::AddIterators(const ReadOptions& read_options, VersionStorageInfo::VersionStorageInfo( const InternalKeyComparator* internal_comparator, const Comparator* user_comparator, int levels, - CompactionStyle compaction_style, VersionStorageInfo* ref_vstorage) + CompactionStyle compaction_style, VersionStorageInfo* ref_vstorage, + bool _force_consistency_checks) : internal_comparator_(internal_comparator), user_comparator_(user_comparator), // cfd is nullptr if Version is dummy @@ -870,7 +871,8 @@ VersionStorageInfo::VersionStorageInfo( current_num_deletions_(0), current_num_samples_(0), estimated_compaction_needed_bytes_(0), - finalized_(false) { + finalized_(false), + force_consistency_checks_(_force_consistency_checks) { if (ref_vstorage != nullptr) { accumulated_file_size_ = ref_vstorage->accumulated_file_size_; accumulated_raw_key_size_ = ref_vstorage->accumulated_raw_key_size_; @@ -894,14 +896,16 @@ Version::Version(ColumnFamilyData* column_family_data, VersionSet* vset, table_cache_((cfd_ == nullptr) ? nullptr : cfd_->table_cache()), merge_operator_((cfd_ == nullptr) ? nullptr : cfd_->ioptions()->merge_operator), - storage_info_((cfd_ == nullptr) ? nullptr : &cfd_->internal_comparator(), - (cfd_ == nullptr) ? nullptr : cfd_->user_comparator(), - cfd_ == nullptr ? 0 : cfd_->NumberLevels(), - cfd_ == nullptr ? kCompactionStyleLevel - : cfd_->ioptions()->compaction_style, - (cfd_ == nullptr || cfd_->current() == nullptr) - ? nullptr - : cfd_->current()->storage_info()), + storage_info_( + (cfd_ == nullptr) ? nullptr : &cfd_->internal_comparator(), + (cfd_ == nullptr) ? nullptr : cfd_->user_comparator(), + cfd_ == nullptr ? 0 : cfd_->NumberLevels(), + cfd_ == nullptr ? kCompactionStyleLevel + : cfd_->ioptions()->compaction_style, + (cfd_ == nullptr || cfd_->current() == nullptr) + ? nullptr + : cfd_->current()->storage_info(), + cfd_ == nullptr ? false : cfd_->ioptions()->force_consistency_checks), vset_(vset), next_(this), prev_(this), diff --git a/db/version_set.h b/db/version_set.h index a2dd3afc3..2e995227d 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -93,7 +93,8 @@ class VersionStorageInfo { VersionStorageInfo(const InternalKeyComparator* internal_comparator, const Comparator* user_comparator, int num_levels, CompactionStyle compaction_style, - VersionStorageInfo* src_vstorage); + VersionStorageInfo* src_vstorage, + bool _force_consistency_checks); ~VersionStorageInfo(); void Reserve(int level, size_t size) { files_[level].reserve(size); } @@ -331,6 +332,8 @@ class VersionStorageInfo { estimated_compaction_needed_bytes_ = v; } + bool force_consistency_checks() const { return force_consistency_checks_; } + private: const InternalKeyComparator* internal_comparator_; const Comparator* user_comparator_; @@ -413,6 +416,10 @@ class VersionStorageInfo { bool finalized_; + // If set to true, we will run consistency checks even if RocksDB + // is compiled in release mode + bool force_consistency_checks_; + friend class Version; friend class VersionSet; // No copying allowed diff --git a/db/version_set_test.cc b/db/version_set_test.cc index 3a1764fba..adafe9c80 100644 --- a/db/version_set_test.cc +++ b/db/version_set_test.cc @@ -110,7 +110,7 @@ class VersionStorageInfoTest : public testing::Test { options_(GetOptionsWithNumLevels(6, logger_)), ioptions_(options_), mutable_cf_options_(options_), - vstorage_(&icmp_, ucmp_, 6, kCompactionStyleLevel, nullptr) {} + vstorage_(&icmp_, ucmp_, 6, kCompactionStyleLevel, nullptr, false) {} ~VersionStorageInfoTest() { for (int i = 0; i < vstorage_.num_levels(); i++) { diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index fbc3e418e..4ce704a63 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -796,6 +796,12 @@ struct ColumnFamilyOptions { // Default: false bool paranoid_file_checks; + // In debug mode, RocksDB run consistency checks on the LSM everytime the LSM + // change (Flush, Compaction, AddFile). These checks are disabled in release + // mode, use this option to enable them in release mode as well. + // Default: false + bool force_consistency_checks; + // Measure IO stats in compactions and flushes, if true. // Default: false bool report_bg_io_stats; diff --git a/util/cf_options.cc b/util/cf_options.cc index a57670ec7..da163f0e2 100644 --- a/util/cf_options.cc +++ b/util/cf_options.cc @@ -69,6 +69,7 @@ ImmutableCFOptions::ImmutableCFOptions(const ImmutableDBOptions& db_options, compaction_readahead_size(db_options.compaction_readahead_size), num_levels(cf_options.num_levels), optimize_filters_for_hits(cf_options.optimize_filters_for_hits), + force_consistency_checks(cf_options.force_consistency_checks), listeners(db_options.listeners), row_cache(db_options.row_cache), max_subcompactions(db_options.max_subcompactions) {} diff --git a/util/cf_options.h b/util/cf_options.h index 0329167a9..e3907fa0d 100644 --- a/util/cf_options.h +++ b/util/cf_options.h @@ -108,6 +108,8 @@ struct ImmutableCFOptions { bool optimize_filters_for_hits; + bool force_consistency_checks; + // A vector of EventListeners which call-back functions will be called // when specific RocksDB event happens. std::vector> listeners; diff --git a/util/options.cc b/util/options.cc index 87068bd7e..36ea3275e 100644 --- a/util/options.cc +++ b/util/options.cc @@ -83,6 +83,7 @@ ColumnFamilyOptions::ColumnFamilyOptions() min_partial_merge_operands(2), optimize_filters_for_hits(false), paranoid_file_checks(false), + force_consistency_checks(false), report_bg_io_stats(false) { assert(memtable_factory.get() != nullptr); } @@ -149,6 +150,7 @@ ColumnFamilyOptions::ColumnFamilyOptions(const Options& options) min_partial_merge_operands(options.min_partial_merge_operands), optimize_filters_for_hits(options.optimize_filters_for_hits), paranoid_file_checks(options.paranoid_file_checks), + force_consistency_checks(options.force_consistency_checks), report_bg_io_stats(options.report_bg_io_stats) { assert(memtable_factory.get() != nullptr); if (max_bytes_for_level_multiplier_additional.size() < @@ -559,6 +561,8 @@ void ColumnFamilyOptions::Dump(Logger* log) const { optimize_filters_for_hits); Header(log, " Options.paranoid_file_checks: %d", paranoid_file_checks); + Header(log, " Options.force_consistency_checks: %d", + force_consistency_checks); Header(log, " Options.report_bg_io_stats: %d", report_bg_io_stats); } // ColumnFamilyOptions::Dump diff --git a/util/options_helper.h b/util/options_helper.h index 2a8c96108..3f856fee9 100644 --- a/util/options_helper.h +++ b/util/options_helper.h @@ -374,6 +374,9 @@ static std::unordered_map cf_options_type_info = { {offsetof(struct ColumnFamilyOptions, paranoid_file_checks), OptionType::kBoolean, OptionVerificationType::kNormal, true, offsetof(struct MutableCFOptions, paranoid_file_checks)}}, + {"force_consistency_checks", + {offsetof(struct ColumnFamilyOptions, force_consistency_checks), + OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}}, {"purge_redundant_kvs_while_flush", {offsetof(struct ColumnFamilyOptions, purge_redundant_kvs_while_flush), OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}}, diff --git a/util/options_settable_test.cc b/util/options_settable_test.cc index d9e86b4ef..0afd2d85a 100644 --- a/util/options_settable_test.cc +++ b/util/options_settable_test.cc @@ -418,6 +418,7 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "merge_operator=aabcxehazrMergeOperator;" "memtable_prefix_bloom_size_ratio=0.4642;" "paranoid_file_checks=true;" + "force_consistency_checks=true;" "inplace_update_num_locks=7429;" "optimize_filters_for_hits=false;" "level_compaction_dynamic_level_bytes=false;" diff --git a/util/testutil.cc b/util/testutil.cc index ec37e0415..3e8c4f42c 100644 --- a/util/testutil.cc +++ b/util/testutil.cc @@ -302,6 +302,7 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, Random* rnd) { cf_opt->paranoid_file_checks = rnd->Uniform(2); cf_opt->purge_redundant_kvs_while_flush = rnd->Uniform(2); cf_opt->verify_checksums_in_compaction = rnd->Uniform(2); + cf_opt->force_consistency_checks = rnd->Uniform(2); // double options cf_opt->hard_rate_limit = static_cast(rnd->Uniform(10000)) / 13; -- GitLab