Fixing endless loop if seeking to end of key with seq num 0

Summary:
When seeking to the last occurrence of a key with sequence number 0, db_iter
ends up in an endless loop because it seeks to type kValueTypeForSeek
which is larger than kTypeDeletion/kTypeValue. Added test case that triggers
the behavior.

Test Plan: make clean all check

Reviewers: igor, rven, anthony, yhchiang, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D43653

diff --git a/db/db_iter.cc b/db/db_iter.cc
index 6ef2353d55f2e50c390d38e904cd65f492224074..7821ffa687b007958efb1a25d3d9e3ad651a73e0 100644 (file)
--- a/db/db_iter.cc
+++ b/db/db_iter.cc
@@ -256,12 +256,13 @@ void DBIter::FindNextUserEntryInternal(bool skipping) {
     // If we have sequentially iterated via numerous keys and still not
     // found the next user-key, then it is better to seek so that we can
     // avoid too many key comparisons. We seek to the last occurrence of
-    // our current key by looking for sequence number 0.
+    // our current key by looking for sequence number 0 and type deletion
+    // (the smallest type).
     if (skipping && num_skipped > max_skip_) {
       num_skipped = 0;
       std::string last_key;
       AppendInternalKey(&last_key, ParsedInternalKey(saved_key_.GetKey(), 0,
-                                                     kValueTypeForSeek));
+                                                     kTypeDeletion));
       iter_->Seek(last_key);
       RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);
     } else {

diff --git a/db/db_iter_test.cc b/db/db_iter_test.cc
index dce8c8282c897267ea431610334d6d581a132f76..5fa5c5c820b97e982b757c26aeab12b6e6889998 100644 (file)
--- a/db/db_iter_test.cc
+++ b/db/db_iter_test.cc
@@ -1644,6 +1644,7 @@ TEST_F(DBIteratorTest, DBIterator7) {
     ASSERT_TRUE(!db_iter->Valid());
   }
 }
+
 TEST_F(DBIteratorTest, DBIterator8) {
   Options options;
   options.merge_operator = MergeOperators::CreateFromStringId("stringappend");
@@ -1747,6 +1748,30 @@ TEST_F(DBIteratorTest, DBIterator10) {
   ASSERT_EQ(db_iter->value().ToString(), "3");
 }
 
+TEST_F(DBIteratorTest, SeekToLastOccurrenceSeq0) {
+  Options options;
+  options.merge_operator = nullptr;
+
+  TestIterator* internal_iter = new TestIterator(BytewiseComparator());
+  internal_iter->AddPut("a", "1");
+  internal_iter->AddPut("b", "2");
+  internal_iter->Finish();
+
+  std::unique_ptr<Iterator> db_iter(NewDBIterator(
+      env_, ImmutableCFOptions(options), BytewiseComparator(), internal_iter,
+      10, 0 /* force seek */));
+  db_iter->SeekToFirst();
+  ASSERT_TRUE(db_iter->Valid());
+  ASSERT_EQ(db_iter->key().ToString(), "a");
+  ASSERT_EQ(db_iter->value().ToString(), "1");
+  db_iter->Next();
+  ASSERT_TRUE(db_iter->Valid());
+  ASSERT_EQ(db_iter->key().ToString(), "b");
+  ASSERT_EQ(db_iter->value().ToString(), "2");
+  db_iter->Next();
+  ASSERT_FALSE(db_iter->Valid());
+}
+
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {