Skip to content

Commit

Permalink
Fixing endless loop if seeking to end of key with seq num 0
Browse files Browse the repository at this point in the history
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
  • Loading branch information
4tXJ7f committed Aug 6, 2015
1 parent 48e6e9a commit d7314ba
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
5 changes: 3 additions & 2 deletions db/db_iter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
25 changes: 25 additions & 0 deletions db/db_iter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit d7314ba

Please sign in to comment.