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.

Sign up for GitHub

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

Jump to bottom

storage: implement RangeKeyChanged for MVCCIncrementalIterator #86105

Closed
erikgrinaker opened this issue Aug 14, 2022 · 1 comment · Fixed by #86259
Closed

storage: implement RangeKeyChanged for MVCCIncrementalIterator #86105

erikgrinaker opened this issue Aug 14, 2022 · 1 comment · Fixed by #86259
Assignees
@erikgrinaker
Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-performance Perf of queries or internals. Solution not expected to change functional behavior.

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Aug 14, 2022 •
edited by exalate-issue-sync bot
Loading

The RangeKeyChanged() iterator method can allow for significant performance improvements when iterating across spans and detecting MVCC range keys. However, this hasn't been implemented for MVCCIncrementalIterator, because of two complications:

  1. The iterator can hide range keys, so even though the range key may change in the underlying iterator, it may not in the incremental iterator (because the range keys were never exposed).

  2. NextIgnoringTime() and NextKeyIgnoringTime() may cause the exposed range keys to change -- both by revealing a previously hidden range key span, and by revealing additional versions for currently exposed range keys. These should most likely cause RangeKeyChanged() to trigger, as should the reversal to regular time-bound iteration.

Jira issue: CRDB-18576

Epic CRDB-2624
@erikgrinaker erikgrinaker added C-performance Perf of queries or internals. Solution not expected to change functional behavior. T-kv-replication labels Aug 14, 2022
@blathers-crl
Copy link

blathers-crl bot commented Aug 14, 2022

cc @cockroachdb/replication

@blathers-crl blathers-crl bot added the A-kv-replication Relating to Raft, consensus, and coordination. label Aug 14, 2022
@erikgrinaker erikgrinaker mentioned this issue Aug 16, 2022
Merged
@craig craig bot closed this as completed in 9c7db33 Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@erikgrinaker erikgrinaker

Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

storage: implement RangeKeyChanged() for MVCCIncrementalIterator erikgrinaker/cockroach
1 participant
@erikgrinaker