From ffacaaa3ea4c9ab62114ee72eaa300ed61ae65dd Mon Sep 17 00:00:00 2001 From: zhangjinpeng1987 Date: Wed, 29 Nov 2017 22:48:55 -0800 Subject: [PATCH] fix Seek with lower_bound Summary: When Seek a key less than `lower_bound`, should return `lower_bound`. ajkr PTAL Closes https://github.com/facebook/rocksdb/pull/3199 Differential Revision: D6421126 Pulled By: ajkr fbshipit-source-id: a06c825830573e0040630704f6bcb3f7f48626f7 --- db/db_iter.cc | 14 ++++++++++++++ db/db_iter_test.cc | 31 +++++++++++++++++++++++++++++-- db/db_iterator_test.cc | 4 ++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/db/db_iter.cc b/db/db_iter.cc index 825a6b840..b070c7caa 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -1134,6 +1134,13 @@ void DBIter::Seek(const Slice& target) { saved_key_.Clear(); saved_key_.SetInternalKey(target, sequence_); + if (iterate_lower_bound_ != nullptr && + user_comparator_->Compare(saved_key_.GetUserKey(), + *iterate_lower_bound_) < 0) { + saved_key_.Clear(); + saved_key_.SetInternalKey(*iterate_lower_bound_, sequence_); + } + { PERF_TIMER_GUARD(seek_internal_seek_time); iter_->Seek(saved_key_.GetInternalKey()); @@ -1177,6 +1184,13 @@ void DBIter::SeekForPrev(const Slice& target) { saved_key_.SetInternalKey(target, 0 /* sequence_number */, kValueTypeForSeekForPrev); + if (iterate_upper_bound_ != nullptr && + user_comparator_->Compare(saved_key_.GetUserKey(), + *iterate_upper_bound_) >= 0) { + saved_key_.Clear(); + saved_key_.SetInternalKey(*iterate_upper_bound_, kMaxSequenceNumber); + } + { PERF_TIMER_GUARD(seek_internal_seek_time); iter_->SeekForPrev(saved_key_.GetInternalKey()); diff --git a/db/db_iter_test.cc b/db/db_iter_test.cc index b4a5f3a58..e1f93db85 100644 --- a/db/db_iter_test.cc +++ b/db/db_iter_test.cc @@ -356,7 +356,7 @@ TEST_F(DBIteratorTest, DBIteratorPrevNext) { db_iter->SeekToLast(); ASSERT_TRUE(db_iter->Valid()); - ASSERT_EQ(static_cast(get_perf_context()->internal_key_skipped_count), 7); + ASSERT_EQ(static_cast(get_perf_context()->internal_key_skipped_count), 1); ASSERT_EQ(db_iter->key().ToString(), "b"); SetPerfLevel(kDisable); @@ -475,7 +475,7 @@ TEST_F(DBIteratorTest, DBIteratorPrevNext) { db_iter->SeekToLast(); ASSERT_TRUE(db_iter->Valid()); - ASSERT_EQ(static_cast(get_perf_context()->internal_delete_skipped_count), 1); + ASSERT_EQ(static_cast(get_perf_context()->internal_delete_skipped_count), 0); ASSERT_EQ(db_iter->key().ToString(), "b"); SetPerfLevel(kDisable); @@ -2991,6 +2991,33 @@ TEST_F(DBIteratorTest, PrevLowerBound) { ASSERT_FALSE(db_iter->Valid()); } +TEST_F(DBIteratorTest, SeekLessLowerBound) { + const int kNumKeys = 3; + const int kLowerBound = 2; + TestIterator* internal_iter = new TestIterator(BytewiseComparator()); + for (int j = 1; j <= kNumKeys; ++j) { + internal_iter->AddPut(std::to_string(j), "val"); + } + internal_iter->Finish(); + + ReadOptions ro; + auto lower_bound_str = std::to_string(kLowerBound); + Slice lower_bound(lower_bound_str); + ro.iterate_lower_bound = &lower_bound; + Options options; + std::unique_ptr db_iter(NewDBIterator( + env_, ro, ImmutableCFOptions(options), BytewiseComparator(), + internal_iter, 10 /* sequence */, + options.max_sequential_skip_in_iterations, nullptr /* read_callback */)); + + auto before_lower_bound_str = std::to_string(kLowerBound - 1); + Slice before_lower_bound(lower_bound_str); + + db_iter->Seek(before_lower_bound); + ASSERT_TRUE(db_iter->Valid()); + ASSERT_EQ(lower_bound_str, db_iter->key().ToString()); +} + } // namespace rocksdb int main(int argc, char** argv) { diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 746946a28..3df8e8c10 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -2152,8 +2152,8 @@ TEST_F(DBIteratorTest, SkipStatistics) { } ASSERT_EQ(count, 2); delete iter; - // 3 deletes + 3 original keys + 2 keys of "b" + lower sequence of "a" - skip_count += 9; + // 3 deletes + 3 original keys + lower sequence of "a" + skip_count += 7; ASSERT_EQ(skip_count, TestGetTickerCount(options, NUMBER_ITER_SKIP)); }