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
zhsj · Oct 8, 2016 · 2ad68b9 · 2ad68b9
1 parent 67501cf
commit 2ad68b9
Show file tree

Hide file tree

Showing 13 changed files with 85 additions and 34 deletions.
diff --git 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
@@ -127,39 +127,62 @@ 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);
       for (size_t i = 1; i < level_files.size(); i++) {
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<std::shared_ptr<EventListener>> listeners;

diff --git 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
@@ -374,6 +374,9 @@ static std::unordered_map<std::string, OptionTypeInfo> 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
@@ -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
@@ -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<double>(rnd->Uniform(10000)) / 13;