From dac5b248b119b8645db0ed4a8bbf1cd1e259dd09 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Fri, 13 Nov 2015 12:01:52 -0800
Subject: [PATCH] UniversalCompactionPicker::PickCompaction(): avoid to form
 compactions if there is no file

Summary:
Currently RocksDB may break in lines like this:

for (size_t i = sorted_runs.size() - 1; i >= first_index_after; i--) {

if options.level0_file_num_compaction_trigger=0.

Fix it by not executing the logic of picking compactions if there is no file (sorted_runs.size() = 0). Also internally set options.level0_file_num_compaction_trigger=1 if users give a 0. 0 is a value makes no sense in RocksDB.

Test Plan: Run all tests. Will add a unit test too.

Reviewers: yhchiang, IslamAbdelRahman, anthony, kradhakrishnan, rven

Reviewed By: rven

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D50727
---
 db/column_family.cc                |  6 +++++
 db/compaction_picker.cc            |  5 ++--
 db/db_universal_compaction_test.cc | 38 ++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/db/column_family.cc b/db/column_family.cc
index f6233a729..60baac775 100644
--- a/db/column_family.cc
+++ b/db/column_family.cc
@@ -175,6 +175,12 @@ ColumnFamilyOptions SanitizeOptions(const DBOptions& db_options,
     result.level0_stop_writes_trigger = std::numeric_limits<int>::max();
   }
 
+  if (result.level0_file_num_compaction_trigger == 0) {
+    Warn(db_options.info_log.get(),
+         "level0_file_num_compaction_trigger cannot be 0");
+    result.level0_file_num_compaction_trigger = 1;
+  }
+
   if (result.level0_stop_writes_trigger <
           result.level0_slowdown_writes_trigger ||
       result.level0_slowdown_writes_trigger <
diff --git a/db/compaction_picker.cc b/db/compaction_picker.cc
index 1d968fc06..d27bd3cb5 100644
--- a/db/compaction_picker.cc
+++ b/db/compaction_picker.cc
@@ -1248,8 +1248,9 @@ Compaction* UniversalCompactionPicker::PickCompaction(
   std::vector<SortedRun> sorted_runs =
       CalculateSortedRuns(*vstorage, ioptions_);
 
-  if (sorted_runs.size() <
-      (unsigned int)mutable_cf_options.level0_file_num_compaction_trigger) {
+  if (sorted_runs.size() == 0 ||
+      sorted_runs.size() <
+          (unsigned int)mutable_cf_options.level0_file_num_compaction_trigger) {
     LogToBuffer(log_buffer, "[%s] Universal: nothing to do\n", cf_name.c_str());
     return nullptr;
   }
diff --git a/db/db_universal_compaction_test.cc b/db/db_universal_compaction_test.cc
index 99091227d..bc18a49b9 100644
--- a/db/db_universal_compaction_test.cc
+++ b/db/db_universal_compaction_test.cc
@@ -112,6 +112,44 @@ class DelayFilterFactory : public CompactionFilterFactory {
 };
 }  // namespace
 
+// Make sure we don't trigger a problem if the trigger conditon is given
+// to be 0, which is invalid.
+TEST_P(DBTestUniversalCompaction, UniversalCompactionSingleSortedRun) {
+  Options options;
+  options = CurrentOptions(options);
+
+  options.compaction_style = kCompactionStyleUniversal;
+  options.num_levels = num_levels_;
+  // Config universal compaction to always compact to one single sorted run.
+  options.level0_file_num_compaction_trigger = 0;
+  options.compaction_options_universal.size_ratio = 10;
+  options.compaction_options_universal.min_merge_width = 2;
+  options.compaction_options_universal.max_size_amplification_percent = 1;
+
+  options.write_buffer_size = 105 << 10;  // 105KB
+  options.arena_block_size = 4 << 10;
+  options.target_file_size_base = 32 << 10;  // 32KB
+  // trigger compaction if there are >= 4 files
+  KeepFilterFactory* filter = new KeepFilterFactory(true);
+  filter->expect_manual_compaction_.store(false);
+  options.compaction_filter_factory.reset(filter);
+
+  DestroyAndReopen(options);
+  ASSERT_EQ(1, db_->GetOptions().level0_file_num_compaction_trigger);
+
+  Random rnd(301);
+  int key_idx = 0;
+
+  filter->expect_full_compaction_.store(true);
+
+  for (int num = 0; num < 16; num++) {
+    // Write 100KB file. And immediately it should be compacted to one file.
+    GenerateNewFile(&rnd, &key_idx);
+    dbfull()->TEST_WaitForCompact();
+    ASSERT_EQ(NumSortedRuns(0), 1);
+  }
+}
+
 // TODO(kailiu) The tests on UniversalCompaction has some issues:
 //  1. A lot of magic numbers ("11" or "12").
 //  2. Made assumption on the memtable flush conditions, which may change from