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 merge operand reappearing when covered by DeleteRange #4481

Closed
wants to merge 1 commit into from

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented Oct 10, 2018

Even during DBIter::Prev(), there is a case where we need to use RangeDelPositioningMode::kForwardTraversal. In particular, when we hit too many internal keys for a single user key, we use seek to find the newest internal key. If it's a merge operand, we then scan forwards, collecting the merge operands. This forward scan should be using RangeDelPositioningMode::kForwardTraversal.

Test Plan: new test case

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.

ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ajkr ajkr force-pushed the range-del-fix-iter-prev-merge branch from 2de7913 to 585457a Compare October 10, 2018 23:30
@facebook-github-bot
Copy link
Contributor

@ajkr has updated the pull request. Re-import the pull request

@ajkr ajkr requested a review from abhimadan October 10, 2018 23:30
Copy link
Contributor

@abhimadan abhimadan left a comment

Choose a reason for hiding this comment

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

Thanks!

@ajkr
Copy link
Contributor Author

ajkr commented Oct 11, 2018

will leave it out of HISTORY.md since we don't think it was a problem before #4432, which is unreleased (though if we backport that we need to remember to backport this too).

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.

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

petermattis pushed a commit to petermattis/rocksdb that referenced this pull request Oct 17, 2018
)

Summary:
Even during `DBIter::Prev()`, there is a case where we need to use `RangeDelPositioningMode::kForwardTraversal`. In particular, when we hit too many internal keys for a single user key, we use seek to find the newest internal key. If it's a merge operand, we then scan forwards, collecting the merge operands. This forward scan should be using `RangeDelPositioningMode::kForwardTraversal`.
Pull Request resolved: facebook#4481

Differential Revision: D10319507

Pulled By: ajkr

fbshipit-source-id: b5ce7352461f3a7696b28a5136ae0076f2bde51f
petermattis pushed a commit to cockroachdb/rocksdb that referenced this pull request Dec 5, 2018
)

Summary:
Even during `DBIter::Prev()`, there is a case where we need to use `RangeDelPositioningMode::kForwardTraversal`. In particular, when we hit too many internal keys for a single user key, we use seek to find the newest internal key. If it's a merge operand, we then scan forwards, collecting the merge operands. This forward scan should be using `RangeDelPositioningMode::kForwardTraversal`.
Pull Request resolved: facebook#4481

Differential Revision: D10319507

Pulled By: ajkr

fbshipit-source-id: b5ce7352461f3a7696b28a5136ae0076f2bde51f
@ajkr ajkr mentioned this pull request Jun 26, 2019
32 tasks
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.

3 participants