Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

yiwu-arbug
Copy link
Contributor

Summary:
Since #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().

Test Plan:
See the new test.

Signed-off-by: Yi Wu yiwu@pingcap.com

Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Copy link
Contributor Author

cc @siying

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@siying is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 4f66ec9.

@yiwu-arbug yiwu-arbug deleted the bound_valid branch July 25, 2019 20:31
merryChris pushed a commit to merryChris/rocksdb that referenced this pull request Nov 18, 2019
…ok#5540)

Summary:
Since facebook#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: facebook#5540

Test Plan:
See the new test.

Signed-off-by: Yi Wu <yiwu@pingcap.com>

Differential Revision: D16127653

fbshipit-source-id: a0691e1164658d485c17971aaa97028812f74678
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants