andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix lower bound check error when iterate across file boundary (#5540)

Browse Source 
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 <yiwu@pingcap.com>

Differential Revision: D16127653

fbshipit-source-id: a0691e1164658d485c17971aaa97028812f74678
This commit is contained in:
Yi Wu 2019-07-04 17:24:33 -07:00 committed by Facebook Github Bot
parent e4dcf5fd22
commit 4f66ec977d
2 changed files with 28 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
db/db_iterator_test.cc
View File

@ -2821,6 +2821,32 @@ TEST_P(DBIteratorTest, IterateBoundChangedBeforeSeek) {
delete iter; 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<Iterator> 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, INSTANTIATE_TEST_CASE_P(DBIteratorTestInstance, DBIteratorTest,
testing::Values(true, false)); testing::Values(true, false));

3
db/version_set.cc
View File

@ -984,7 +984,8 @@ class LevelIterator final : public InternalIterator {
// Note MyRocks may update iterate bounds between seek. To workaround it, // Note MyRocks may update iterate bounds between seek. To workaround it,
// we need to check and update may_be_out_of_lower_bound_ accordingly. // we need to check and update may_be_out_of_lower_bound_ accordingly.
void CheckMayBeOutOfLowerBound() { 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_ = may_be_out_of_lower_bound_ =
user_comparator_.Compare( user_comparator_.Compare(
ExtractUserKey(file_smallest_key(file_index_)), ExtractUserKey(file_smallest_key(file_index_)),