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: ensure readAsOfIterator caches rangekeys #84714

Closed
msbutler opened this issue Jul 20, 2022 · 2 comments · Fixed by #86513
Closed

storage: ensure readAsOfIterator caches rangekeys #84714

msbutler opened this issue Jul 20, 2022 · 2 comments · Fixed by #86513
Assignees
@erikgrinaker
Labels
A-disaster-recovery A-kv-replication Relating to Raft, consensus, and coordination. branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-disaster-recovery

Comments

@msbutler
Copy link
Collaborator

msbutler commented Jul 20, 2022 •
edited by exalate-issue-sync bot
Loading

Currently, the readAsOfIterator calls the expensive f.iter.RangeKeys() every time f.iter.HasPointAndRange == (true,true). Consequently, if RESTORE encounters a range key, it faces a steep performance hit.

Instead, if the range bounds during iteration remain the same, the range keys should be cached. This should be solved by #84379, but if that doesn't land by 22.2, then the readAsOfIterator will need to manually cache range keys.

Jira issue: CRDB-17824

Epic CRDB-2624
@msbutler msbutler added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 labels Jul 20, 2022
@msbutler msbutler self-assigned this Jul 20, 2022
@msbutler msbutler mentioned this issue Jul 20, 2022
Merged
@blathers-crl
Copy link

blathers-crl bot commented Jul 20, 2022

cc @cockroachdb/replication

@blathers-crl blathers-crl bot added the A-kv-replication Relating to Raft, consensus, and coordination. label Jul 20, 2022
@blathers-crl
Copy link

blathers-crl bot commented Aug 3, 2022

cc @cockroachdb/bulk-io

@erikgrinaker erikgrinaker mentioned this issue Aug 20, 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-disaster-recovery A-kv-replication Relating to Raft, consensus, and coordination. branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-disaster-recovery
Projects
No open projects
Disaster Recovery Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

storage: use RangeKeyChanged in ReadAsOfIterator erikgrinaker/cockroach
2 participants
@erikgrinaker @msbutler