From 43a340bebe28a5ee3c17ecd840430e2d95f1f2e0 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Tue, 17 Sep 2019 17:08:57 -0700
Subject: [PATCH] Merging iterator to disble reseek optimization in prefix seek
 (#5815)

Summary:
We are seeing a bug of wrong results with merging iterator's reseek avoidence feature and prefix extractor. Disable this optimization for now.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5815

Test Plan: Validated the same MyRocks case was fixed; run all existing tests.

Differential Revision: D17430776

fbshipit-source-id: aef664277ba0ab8a2e68331ff0db6ae682535371
---
 db/db_test2.cc            | 40 +++++++++++++++++++++++++++++++++++++++
 table/merging_iterator.cc |  2 +-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/db/db_test2.cc b/db/db_test2.cc
index a4c5b5aa3..746966bda 100644
--- a/db/db_test2.cc
+++ b/db/db_test2.cc
@@ -3987,6 +3987,46 @@ TEST_F(DBTest2, CloseWithUnreleasedSnapshot) {
   db_ = nullptr;
 }
 
+TEST_F(DBTest2, PrefixBloomReseek) {
+  Options options = CurrentOptions();
+  options.create_if_missing = true;
+  options.prefix_extractor.reset(NewCappedPrefixTransform(3));
+  BlockBasedTableOptions bbto;
+  bbto.filter_policy.reset(NewBloomFilterPolicy(10, false));
+  bbto.whole_key_filtering = false;
+  options.table_factory.reset(NewBlockBasedTableFactory(bbto));
+  DestroyAndReopen(options);
+
+  // Construct two L1 files with keys:
+  // f1:[aaa1 ccc1] f2:[ddd0]
+  ASSERT_OK(Put("aaa1", ""));
+  ASSERT_OK(Put("ccc1", ""));
+  ASSERT_OK(Flush());
+  ASSERT_OK(Put("ddd0", ""));
+  ASSERT_OK(Flush());
+  CompactRangeOptions cro;
+  cro.bottommost_level_compaction = BottommostLevelCompaction::kSkip;
+  ASSERT_OK(db_->CompactRange(cro, nullptr, nullptr));
+
+  ASSERT_OK(Put("bbb1", ""));
+
+  Iterator* iter = db_->NewIterator(ReadOptions());
+
+  // Seeking into f1, the iterator will check bloom filter which returns the
+  // file iterator ot be invalidate, and the cursor will put into f2, with
+  // the next key to be "ddd0".
+  iter->Seek("bbb1");
+  ASSERT_TRUE(iter->Valid());
+  ASSERT_EQ("bbb1", iter->key().ToString());
+
+  // Reseek ccc1, the L1 iterator needs to go back to f1 and reseek.
+  iter->Seek("ccc1");
+  ASSERT_TRUE(iter->Valid());
+  ASSERT_EQ("ccc1", iter->key().ToString());
+
+  delete iter;
+}
+
 #ifndef ROCKSDB_LITE
 TEST_F(DBTest2, RowCacheSnapshot) {
   Options options = CurrentOptions();
diff --git a/table/merging_iterator.cc b/table/merging_iterator.cc
index 82329ae8a..62d5ce1ee 100644
--- a/table/merging_iterator.cc
+++ b/table/merging_iterator.cc
@@ -111,7 +111,7 @@ class MergingIterator : public InternalIterator {
   void Seek(const Slice& target) override {
     bool is_increasing_reseek = false;
     if (current_ != nullptr && direction_ == kForward && status_.ok() &&
-        comparator_->Compare(target, key()) >= 0) {
+        !prefix_seek_mode_ && comparator_->Compare(target, key()) >= 0) {
       is_increasing_reseek = true;
     }
     ClearHeaps();