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()"
  ec70fea4c4.

* "Fixed endless loop in DBIter::FindPrevUserKey()"
  acee2b08a2.

Test Plan: db_stress

Reviewers: anthony, igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D41301
main
Yueh-Hsuan Chiang 9 years ago
parent e12b403991
commit 685582a0b4
  1. 7
      db/db_iter.cc
  2. 8
      db/db_iter_test.cc
  3. 4
      db/db_test.cc
  4. 13
      table/merger.cc

@ -350,9 +350,6 @@ void DBIter::MergeValuesNewToOld() {
void DBIter::Prev() { void DBIter::Prev() {
assert(valid_); assert(valid_);
if (direction_ == kForward) { if (direction_ == kForward) {
if (!iter_->Valid()) {
iter_->SeekToLast();
}
FindPrevUserKey(); FindPrevUserKey();
direction_ = kReverse; direction_ = kReverse;
} }
@ -556,7 +553,7 @@ void DBIter::FindNextUserKey() {
ParsedInternalKey ikey; ParsedInternalKey ikey;
FindParseableKey(&ikey, kForward); FindParseableKey(&ikey, kForward);
while (iter_->Valid() && 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(); iter_->Next();
FindParseableKey(&ikey, kForward); FindParseableKey(&ikey, kForward);
} }
@ -571,7 +568,7 @@ void DBIter::FindPrevUserKey() {
ParsedInternalKey ikey; ParsedInternalKey ikey;
FindParseableKey(&ikey, kReverse); FindParseableKey(&ikey, kReverse);
while (iter_->Valid() && 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_) { if (num_skipped >= max_skip_) {
num_skipped = 0; num_skipped = 0;
IterKey last_key; IterKey last_key;

@ -1668,7 +1668,9 @@ TEST_F(DBIteratorTest, DBIterator8) {
ASSERT_EQ(db_iter->value().ToString(), "0"); 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 options;
options.merge_operator = MergeOperators::CreateFromStringId("stringappend"); 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; Options options;
TestIterator* internal_iter = new TestIterator(BytewiseComparator()); TestIterator* internal_iter = new TestIterator(BytewiseComparator());

@ -14111,7 +14111,9 @@ TEST_F(DBTest, RowCache) {
ASSERT_EQ(TestGetTickerCount(options, ROW_CACHE_MISS), 1); 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 options;
options.create_if_missing = true; options.create_if_missing = true;
options.merge_operator = MergeOperators::CreatePutOperator(); options.merge_operator = MergeOperators::CreatePutOperator();

@ -179,19 +179,6 @@ class MergingIterator : public Iterator {
} else { } else {
// Child has no entries >= key(). Position at last entry. // Child has no entries >= key(). Position at last entry.
child.SeekToLast(); 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()) { if (child.Valid()) {

Loading…
Cancel
Save