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

internal/keyspan: lift visibility filtering out of Seek{GE,LE} #1687

Merged
merged 1 commit into from
May 18, 2022

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented May 6, 2022

Alter keyspan.SeekGE and keyspan.SeekLE to omit any sequence number
filtering. The caller must now filter a Span's keys if necessary. This avoids
extra work in the case that there are no visible keys overlapping the search
key. Previously SeekGE and SeekLE would next or prev until they found a visible
key. Now, the caller can simply use the empty span.

This is partially motivated by the batch range deletion slowdown observed in
#1675.

@jbowens jbowens requested a review from a team May 6, 2022 17:59
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Alter `keyspan.SeekGE` and `keyspan.SeekLE` to omit any sequence number
filtering. The caller must now filter a Span's keys if necessary. This avoids
extra work in the case that there are no visible keys overlapping the search
key. Previously SeekGE and SeekLE would next or prev until they found a visible
key. Now, the caller can simply use the empty span.

This is partially motivated by the batch range deletion slowdown observed in
impact.
@jbowens jbowens force-pushed the keyspan-seekgele branch from db4aef7 to 4c9483a Compare May 6, 2022 22:16
Copy link
Contributor

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jbowens)

Copy link
Collaborator Author

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jbowens)

@jbowens jbowens merged commit a07efd9 into cockroachdb:master May 18, 2022
@jbowens jbowens deleted the keyspan-seekgele branch May 18, 2022 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants