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
(cherry picked from commit 685582a0b43cb19cb12bf7122657f9ea0f8c52c4)

diff --git a/db/db_iter.cc b/db/db_iter.cc
index 7ed00365e7815d8c2b47a1692066664a1fb14455..6bee646354f4232c0552582a3fad0b40fbd3fe94 100644 (file)
--- 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 e5c58e4d978ca57dbf49a9855eca5c94a1a79672..2cb81aab13644b7a3d06976cf4abc64db670bfd7 100644 (file)
--- 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 ddf7de9a48755b01306989118706ebaeced2f2d7..9a8fe6d9ef2411e5b639bbf82ab3b3712fd88e93 100644 (file)
--- 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 9781d0dcaf44d62b2074df8f0ea0f4e54f1db3d9..b418b88a406c0edca5e48b7a52827f50409f1ce4 100644 (file)
--- a/table/merger.cc
+++ b/table/merger.cc
@@ -208,19 +208,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()) {
             maxHeap_.push(&child);