Fix misplaced position for reversing iterator direction while current key is a merge

Summary:
While doing forward iterating, if current key is merge, internal iterator position is placed to the next key. If Prev() is called now, needs to do extra Prev() to recover the location.
This is second attempt of fixing after reverting ec70fea4c4. This time shrink the fix to only merge key is the current key and avoid the reseeking logic for max_iterating skipping

Test Plan: enable the two disabled tests and make sure they pass

Reviewers: rven, IslamAbdelRahman, kradhakrishnan, tnovak, yhchiang

Reviewed By: yhchiang

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D43557
main
sdong 9 years ago
parent c465071029
commit 8e01bd1144
  1. 31
      db/db_iter.cc
  2. 4
      db/db_iter_test.cc
  3. 2
      db/db_test.cc

@ -115,6 +115,7 @@ class DBIter: public Iterator {
virtual void SeekToLast() override; virtual void SeekToLast() override;
private: private:
void ReverseToBackward();
void PrevInternal(); void PrevInternal();
void FindParseableKey(ParsedInternalKey* ikey, Direction direction); void FindParseableKey(ParsedInternalKey* ikey, Direction direction);
bool FindValueForCurrentKey(); bool FindValueForCurrentKey();
@ -350,12 +351,38 @@ void DBIter::MergeValuesNewToOld() {
void DBIter::Prev() { void DBIter::Prev() {
assert(valid_); assert(valid_);
if (direction_ == kForward) { if (direction_ == kForward) {
FindPrevUserKey(); ReverseToBackward();
direction_ = kReverse;
} }
PrevInternal(); PrevInternal();
} }
void DBIter::ReverseToBackward() {
if (current_entry_is_merged_) {
// Not placed in the same key. Need to call Prev() until finding the
// previous key.
if (!iter_->Valid()) {
iter_->SeekToLast();
}
ParsedInternalKey ikey;
FindParseableKey(&ikey, kReverse);
while (iter_->Valid() &&
user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) > 0) {
iter_->Prev();
FindParseableKey(&ikey, kReverse);
}
}
#ifndef NDEBUG
if (iter_->Valid()) {
ParsedInternalKey ikey;
assert(ParseKey(&ikey));
assert(user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) <= 0);
}
#endif
FindPrevUserKey();
direction_ = kReverse;
}
void DBIter::PrevInternal() { void DBIter::PrevInternal() {
if (!iter_->Valid()) { if (!iter_->Valid()) {
valid_ = false; valid_ = false;

@ -1670,7 +1670,7 @@ TEST_F(DBIteratorTest, DBIterator8) {
// TODO(3.13): fix the issue of Seek() then Prev() which might not necessary // TODO(3.13): fix the issue of Seek() then Prev() which might not necessary
// return the biggest element smaller than the seek key. // return the biggest element smaller than the seek key.
TEST_F(DBIteratorTest, DISABLED_DBIterator9) { TEST_F(DBIteratorTest, DBIterator9) {
Options options; Options options;
options.merge_operator = MergeOperators::CreateFromStringId("stringappend"); options.merge_operator = MergeOperators::CreateFromStringId("stringappend");
{ {
@ -1720,7 +1720,7 @@ TEST_F(DBIteratorTest, DISABLED_DBIterator9) {
// TODO(3.13): fix the issue of Seek() then Prev() which might not necessary // TODO(3.13): fix the issue of Seek() then Prev() which might not necessary
// return the biggest element smaller than the seek key. // return the biggest element smaller than the seek key.
TEST_F(DBIteratorTest, DISABLED_DBIterator10) { TEST_F(DBIteratorTest, DBIterator10) {
Options options; Options options;
TestIterator* internal_iter = new TestIterator(BytewiseComparator()); TestIterator* internal_iter = new TestIterator(BytewiseComparator());

@ -8226,7 +8226,7 @@ TEST_F(DBTest, RowCache) {
// TODO(3.13): fix the issue of Seek() + Prev() which might not necessary // TODO(3.13): fix the issue of Seek() + Prev() which might not necessary
// return the biggest key which is smaller than the seek key. // return the biggest key which is smaller than the seek key.
TEST_F(DBTest, DISABLED_PrevAfterMerge) { TEST_F(DBTest, PrevAfterMerge) {
Options options; Options options;
options.create_if_missing = true; options.create_if_missing = true;
options.merge_operator = MergeOperators::CreatePutOperator(); options.merge_operator = MergeOperators::CreatePutOperator();

Loading…
Cancel
Save