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

backupccl: memory monitored restore processor erroneously restores deleted data #103334

Open
7 tasks
stevendanna opened this issue May 15, 2023 · 1 comment
Open
7 tasks
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery

Comments

@stevendanna
Copy link
Collaborator

stevendanna commented May 15, 2023

Describe the problem

The new memory monitored restored attempts to limit the number of SSTs added to a single iterator. When a given span has more SSTs that would be allowed by the memory monitor, it attempts to process one set of SSTs first, and then the second set.

This, however, poses a problem for deletions. Currently, we do not write deletions directly, rather we depend on the iterator never returning a deleted key. This assumption was correct when all SSTs related to a key were definitely in the same iterator. It is no longer correct when the SSTs for a given iterator can be split over multiple iterators.

Proposed work to fix this

To fix this we think we need to (1) ensure that all SSTs for a given layer are all inside the same iterator and (2) change our usage of the iterator to raise deletion tombstones (both point and range keys) and then explicitly write those deletions during the restore process.

  • Unit test that reproduces the between-layer issue
  • Unit test that reproduces in in-layer issue
  • Pass layer information in RestoreSpanSpec
  • Ensure all SSTs from a single layer are added to a restore iterator
  • Raise point deletion tombstones in ReadAsOfInterator
  • Raise range tombstones in ReadAsOfIterator
  • Correctly write range tombstones during restore

See

Jira issue: CRDB-27949

Epic CRDB-28050

@stevendanna stevendanna added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label May 15, 2023
@blathers-crl
Copy link

blathers-crl bot commented May 15, 2023

cc @cockroachdb/disaster-recovery

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery
Projects
None yet
Development

No branches or pull requests

2 participants