From b5881762bc14dd15702a62716bbd8a31c45fa357 Mon Sep 17 00:00:00 2001
From: Igor Canadi <icanadi@fb.com>
Date: Tue, 12 May 2015 11:16:25 -0700
Subject: [PATCH] Reset parent_index and base_index when picking files marked
 for compaction

Summary: This caused a crash of our MongoDB + RocksDB instance. PickCompactionBySize() sets its own parent_index. We never reset this parent_index when picking PickFilesMarkedForCompactionExperimental(). So we might end up doing SetupOtherInputs() with parent_index that was set by PickCompactionBySize, although we're using compaction calculated using PickFilesMarkedForCompactionExperimental.

Test Plan: Added a unit test that fails with assertion on master.

Reviewers: yhchiang, rven, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D38337
---
 db/compaction_picker.cc      |  1 +
 db/compaction_picker_test.cc | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/db/compaction_picker.cc b/db/compaction_picker.cc
index 09a782182..d32ceb0fa 100644
--- a/db/compaction_picker.cc
+++ b/db/compaction_picker.cc
@@ -796,6 +796,7 @@ Compaction* LevelCompactionPicker::PickCompaction(
   // compaction
   if (inputs.empty()) {
     is_manual = true;
+    parent_index = base_index = -1;
     PickFilesMarkedForCompactionExperimental(cf_name, vstorage, &inputs, &level,
                                              &output_level);
   }
diff --git a/db/compaction_picker_test.cc b/db/compaction_picker_test.cc
index d2a4d5fd0..9efd9510b 100644
--- a/db/compaction_picker_test.cc
+++ b/db/compaction_picker_test.cc
@@ -395,6 +395,28 @@ TEST_F(CompactionPickerTest, NeedsCompactionFIFO) {
   }
 }
 
+// This test exhibits the bug where we don't properly reset parent_index in
+// PickCompaction()
+TEST_F(CompactionPickerTest, ParentIndexResetBug) {
+  int num_levels = ioptions_.num_levels;
+  mutable_cf_options_.level0_file_num_compaction_trigger = 2;
+  mutable_cf_options_.max_bytes_for_level_base = 200;
+  NewVersionStorage(num_levels, kCompactionStyleLevel);
+  Add(0, 1U, "150", "200");       // <- marked for compaction
+  Add(1, 3U, "400", "500", 600);  // <- this one needs compacting
+  Add(2, 4U, "150", "200");
+  Add(2, 5U, "201", "210");
+  Add(2, 6U, "300", "310");
+  Add(2, 7U, "400", "500");  // <- being compacted
+
+  vstorage_->LevelFiles(2)[3]->being_compacted = true;
+  vstorage_->LevelFiles(0)[0]->marked_for_compaction = true;
+
+  UpdateVersionStorageInfo();
+
+  std::unique_ptr<Compaction> compaction(level_compaction_picker.PickCompaction(
+      cf_name_, mutable_cf_options_, vstorage_.get(), &log_buffer_));
+}
 
 }  // namespace rocksdb