From 4f66ec977d9b8b83c0b7e16d25a43281cd6a8073 Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Thu, 4 Jul 2019 17:24:33 -0700 Subject: [PATCH] Fix lower bound check error when iterate across file boundary (#5540) Summary: Since https://github.com/facebook/rocksdb/issues/5468 `LevelIterator` compare lower bound and file smallest key on `NewFileIterator` and cache the result to reduce per key lower bound check. However when iterate across file boundary, it doesn't update the cached result since `Valid()=false` because `Valid()` still reflect the status of the previous file iterator. Fixing it by remove the `Valid()` check from `CheckMayBeOutOfLowerBound()`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5540 Test Plan: See the new test. Signed-off-by: Yi Wu Differential Revision: D16127653 fbshipit-source-id: a0691e1164658d485c17971aaa97028812f74678 --- db/db_iterator_test.cc | 26 ++++++++++++++++++++++++++ db/version_set.cc | 3 ++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 67a97b20b..997b38602 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -2821,6 +2821,32 @@ TEST_P(DBIteratorTest, IterateBoundChangedBeforeSeek) { delete iter; } +TEST_P(DBIteratorTest, IterateWithLowerBoundAcrossFileBoundary) { + ASSERT_OK(Put("aaa", "v")); + ASSERT_OK(Put("bbb", "v")); + ASSERT_OK(Flush()); + ASSERT_OK(Put("ccc", "v")); + ASSERT_OK(Put("ddd", "v")); + ASSERT_OK(Flush()); + // Move both files to bottom level. + ASSERT_OK(dbfull()->CompactRange(CompactRangeOptions(), nullptr, nullptr)); + Slice lower_bound("b"); + ReadOptions read_opts; + read_opts.iterate_lower_bound = &lower_bound; + std::unique_ptr iter(NewIterator(read_opts)); + iter->SeekForPrev("d"); + ASSERT_TRUE(iter->Valid()); + ASSERT_OK(iter->status()); + ASSERT_EQ("ccc", iter->key()); + iter->Prev(); + ASSERT_TRUE(iter->Valid()); + ASSERT_OK(iter->status()); + ASSERT_EQ("bbb", iter->key()); + iter->Prev(); + ASSERT_FALSE(iter->Valid()); + ASSERT_OK(iter->status()); +} + INSTANTIATE_TEST_CASE_P(DBIteratorTestInstance, DBIteratorTest, testing::Values(true, false)); diff --git a/db/version_set.cc b/db/version_set.cc index 226ba0e7e..32dd61db8 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -984,7 +984,8 @@ class LevelIterator final : public InternalIterator { // Note MyRocks may update iterate bounds between seek. To workaround it, // we need to check and update may_be_out_of_lower_bound_ accordingly. void CheckMayBeOutOfLowerBound() { - if (Valid() && read_options_.iterate_lower_bound != nullptr) { + if (read_options_.iterate_lower_bound != nullptr && + file_index_ < flevel_->num_files) { may_be_out_of_lower_bound_ = user_comparator_.Compare( ExtractUserKey(file_smallest_key(file_index_)),