From d7314ba7594f29db58eb3ccd5e16b53b98b5e7cd Mon Sep 17 00:00:00 2001 From: Andres Noetzli Date: Thu, 6 Aug 2015 10:43:28 -0700 Subject: [PATCH] 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 --- db/db_iter.cc | 5 +++-- db/db_iter_test.cc | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/db/db_iter.cc b/db/db_iter.cc index 6ef2353d5..7821ffa68 100644 --- 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 dce8c8282..5fa5c5c82 100644 --- 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 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) {