From 685582a0b43cb19cb12bf7122657f9ea0f8c52c4 Mon Sep 17 00:00:00 2001
From: Yueh-Hsuan Chiang <yhchiang@fb.com>
Date: Tue, 7 Jul 2015 11:36:24 -0700
Subject: [PATCH] Revert two diffs related to DBIter::FindPrevUserKey()

Summary:
This diff reverts the following two previous diffs related to
DBIter::FindPrevUserKey(), which makes db_stress unstable.
We should bake a better fix for this.

* "Fix a comparison in DBIter::FindPrevUserKey()"
  ec70fea4c4025351190eba7a02bd09bb5f083790.

* "Fixed endless loop in DBIter::FindPrevUserKey()"
  acee2b08a2d37154b8f9e2dc74b1966202c15ec5.

Test Plan: db_stress

Reviewers: anthony, igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D41301
---
 db/db_iter.cc      |  7 ++-----
 db/db_iter_test.cc |  8 ++++++--
 db/db_test.cc      |  4 +++-
 table/merger.cc    | 13 -------------
 4 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/db/db_iter.cc b/db/db_iter.cc
index 7ed00365e..6bee64635 100644
--- a/db/db_iter.cc
+++ b/db/db_iter.cc
@@ -350,9 +350,6 @@ void DBIter::MergeValuesNewToOld() {
 void DBIter::Prev() {
   assert(valid_);
   if (direction_ == kForward) {
-    if (!iter_->Valid()) {
-      iter_->SeekToLast();
-    }
     FindPrevUserKey();
     direction_ = kReverse;
   }
@@ -556,7 +553,7 @@ void DBIter::FindNextUserKey() {
   ParsedInternalKey ikey;
   FindParseableKey(&ikey, kForward);
   while (iter_->Valid() &&
-         user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) <= 0) {
+         user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) != 0) {
     iter_->Next();
     FindParseableKey(&ikey, kForward);
   }
@@ -571,7 +568,7 @@ void DBIter::FindPrevUserKey() {
   ParsedInternalKey ikey;
   FindParseableKey(&ikey, kReverse);
   while (iter_->Valid() &&
-         user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) >= 0) {
+         user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) == 0) {
     if (num_skipped >= max_skip_) {
       num_skipped = 0;
       IterKey last_key;
diff --git a/db/db_iter_test.cc b/db/db_iter_test.cc
index e5c58e4d9..2cb81aab1 100644
--- a/db/db_iter_test.cc
+++ b/db/db_iter_test.cc
@@ -1668,7 +1668,9 @@ TEST_F(DBIteratorTest, DBIterator8) {
   ASSERT_EQ(db_iter->value().ToString(), "0");
 }
 
-TEST_F(DBIteratorTest, DBIterator9) {
+// TODO(3.13): fix the issue of Seek() then Prev() which might not necessary
+//             return the biggest element smaller than the seek key.
+TEST_F(DBIteratorTest, DISABLED_DBIterator9) {
   Options options;
   options.merge_operator = MergeOperators::CreateFromStringId("stringappend");
   {
@@ -1716,7 +1718,9 @@ TEST_F(DBIteratorTest, DBIterator9) {
   }
 }
 
-TEST_F(DBIteratorTest, DBIterator10) {
+// TODO(3.13): fix the issue of Seek() then Prev() which might not necessary
+//             return the biggest element smaller than the seek key.
+TEST_F(DBIteratorTest, DISABLED_DBIterator10) {
   Options options;
 
   TestIterator* internal_iter = new TestIterator(BytewiseComparator());
diff --git a/db/db_test.cc b/db/db_test.cc
index ddf7de9a4..9a8fe6d9e 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -14111,7 +14111,9 @@ TEST_F(DBTest, RowCache) {
   ASSERT_EQ(TestGetTickerCount(options, ROW_CACHE_MISS), 1);
 }
 
-TEST_F(DBTest, PrevAfterMerge) {
+// TODO(3.13): fix the issue of Seek() + Prev() which might not necessary
+//             return the biggest key which is smaller than the seek key.
+TEST_F(DBTest, DISABLED_PrevAfterMerge) {
   Options options;
   options.create_if_missing = true;
   options.merge_operator = MergeOperators::CreatePutOperator();
diff --git a/table/merger.cc b/table/merger.cc
index 943a360e9..22886f1d6 100644
--- a/table/merger.cc
+++ b/table/merger.cc
@@ -179,19 +179,6 @@ class MergingIterator : public Iterator {
           } else {
             // Child has no entries >= key().  Position at last entry.
             child.SeekToLast();
-            if (child.Valid() && comparator_->Compare(child.key(), key()) > 0) {
-              // Prefix bloom or prefix hash may return !Valid() if one of the
-              // following condition happens:  1. when prefix doesn't match.
-              // 2. Does not exist any row larger than the key within the prefix
-              // while SeekToLast() may return larger keys.
-              //
-              // Temporarily remove this child to avoid Prev() to return a key
-              // larger than the original keys. However, this can cause missing
-              // rows.
-              //
-              // TODO(3.13): need to fix.
-              continue;
-            }
           }
         }
         if (child.Valid()) {