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

backupccl: create readAsOfMVCCIterator #77276

Closed
msbutler opened this issue Mar 2, 2022 · 1 comment · Fixed by #77281
Closed

backupccl: create readAsOfMVCCIterator #77276

msbutler opened this issue Mar 2, 2022 · 1 comment · Fixed by #77281
Assignees
@msbutler
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-disaster-recovery

Comments

@msbutler
Copy link
Collaborator

msbutler commented Mar 2, 2022 •
edited by cockroach-jira-scripts
Loading

Currently, while ingesting SSTs in the restore processor, processRestoreSpanEntry in restore_data_processor.go manually skips delete tombstones and keys with timestamps later than the AS OF SYSTEM TIME timestamp provided in the RESTORE command.

This logic should be abstracted away from the restore processor. Specifically, when iterating through an SST, processRestoreSpanEntry should use a readAsOfMVCCIterator, an iterator that wraps a SimpleMVCCIterator such that all keys returned are not tombstones and will have timestamps less than the AOST timestamp,

Related to #71155

Epic CRDB-11252

Jira issue: CRDB-13513
@msbutler msbutler added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-disaster-recovery labels Mar 2, 2022
@msbutler msbutler self-assigned this Mar 2, 2022
@blathers-crl
Copy link

blathers-crl bot commented Mar 2, 2022

cc @cockroachdb/bulk-io

msbutler added a commit to msbutler/cockroach that referenced this issue Mar 2, 2022
@msbutler
backupccl: create readAsOfMVCCIterator
4b2ba3c 
Previously while ingesting SSTs in the restore processor,
`processRestoreSpanEntry` would manually skip delete tombstones and keys with
timestamps later than the AS OF SYSTEM TIME timestamp provided in the RESTORE
command.

This PR introduces the readAsOfMVCCIterator which wraps a SimpleMVCCIterator
such that all keys returned are not tombstones and will have timestamps less
than the AOST timestamp. This PR also hooks this iterator into the restore
processor. Some methods in this iterator may get updated as cockroachdb#71155 gets
addressed.

Fixes cockroachdb#77276

Release justification: low risk, high benefit changes to existing functionality

Release note: None
@msbutler msbutler mentioned this issue Mar 2, 2022
Merged
msbutler added a commit to msbutler/cockroach that referenced this issue Mar 2, 2022
@msbutler
backupccl: create readAsOfMVCCIterator
ea04166 
Previously while ingesting SSTs in the restore processor,
`processRestoreSpanEntry` would manually skip delete tombstones and keys with
timestamps later than the AS OF SYSTEM TIME timestamp provided in the RESTORE
command.

This PR introduces the readAsOfMVCCIterator which wraps a SimpleMVCCIterator
such that all keys returned are not tombstones and will have timestamps less
than the AOST timestamp. This PR also hooks this iterator into the restore
processor. Some methods in this iterator may get updated as cockroachdb#71155 gets
addressed.

Fixes cockroachdb#77276

Release justification: low risk, high benefit changes to existing functionality

Release note: None
msbutler added a commit to msbutler/cockroach that referenced this issue Apr 29, 2022
@msbutler
backupccl: create readAsOfMVCCIterator
4a1b2f7 
Previously while ingesting SSTs in the restore processor,
`processRestoreSpanEntry` would manually skip delete tombstones and keys with
timestamps later than the AS OF SYSTEM TIME timestamp provided in the RESTORE
command.

This PR introduces the readAsOfMVCCIterator which wraps a SimpleMVCCIterator
such that all keys returned are not tombstones and will have timestamps less
than the AOST timestamp. This PR also hooks this iterator into the restore
processor. Some methods in this iterator may get updated as cockroachdb#71155 gets
addressed.

Fixes cockroachdb#77276

Release justification: low risk, high benefit changes to existing functionality

Release note: None
msbutler added a commit to msbutler/cockroach that referenced this issue May 24, 2022
@msbutler
backupccl: create readAsOfMVCCIterator
04179e5 
Previously while ingesting SSTs in the restore processor,
`processRestoreSpanEntry` would manually skip delete tombstones and keys with
timestamps later than the AS OF SYSTEM TIME timestamp provided in the RESTORE
command.

This PR introduces the readAsOfMVCCIterator which wraps a SimpleMVCCIterator
such that all keys returned are not tombstones and will have timestamps less
than the AOST timestamp. This PR also hooks this iterator into the restore
processor. Some methods in this iterator may get updated as cockroachdb#71155 gets
addressed.

Fixes cockroachdb#77276

Release justification: low risk, high benefit changes to existing functionality

Release note: None
craig bot pushed a commit that referenced this issue May 25, 2022
@msbutler @adityamaru
@rafiss @rickystewart @yuzefovich
Merge #77281 #81379 #81829 #81841 #81848
fa277e9 
77281: backupccl: create readAsOfIterator for RESTORE r=erikgrinaker a=msbutler

Previously while ingesting SSTs in the restore processor,
`processRestoreSpanEntry` would manually skip delete tombstones and keys with
timestamps later than the AS OF SYSTEM TIME timestamp provided in the RESTORE
command. This PR wraps the restore processor's iterator with a new
readAsOfIterator which abstracts this logic away.

The readAsOfIterator wraps a SimpleMVCCIterator
ond only surfaces the latest valid key of a given MVCC key that is also below
the iterator's AOST timestamp, if set. Further, the iterator does not surface
delete tombstones, nor any MVCC keys shadowed by delete tombstones below the
AOST timestamp, if set.

Fixes #77276

Release note: None

81379: tracing: fix inconsistent lock ordering when notifying event listeners r=andreimatei a=adityamaru

This change fixes an inconsistent lock ordering that was caught in #81361.
As per the lock ordering convention between a parent, and a child span
we now notify the parent of StructuredEvents recorded by the child outside
of the child span's lock.

Additionally, we now notify the event listeners without holding the span's mutex.
This is to protect against deadlocks if the `Notify` method were to call methods
on the tracing span which acquire the span's mutex. One can imagine `Notify` calling
`SetLazyTag` on the same tracing span as the one that has the event
listener.

Fixes: #81361

Release note: None

81829: vendor: update gopgkrb5 to v1.0.3 r=knz a=rafiss

This makes it compatible with both lib/pq and pgx.

Release note: None

81841: build: bump memory allotted to `node` r=celiala,irfansharif a=rickystewart

Since `f032fe85e5ea4cc4c941a4ee8a704b3d2b1eae63` certain builds are
stalling indefinitely. Allotting more memory to `node` resolves the
issue, which mirrors a fix we already have on `release-21.2`.

It is currently unclear why only certain builds stall indefinitely in
this way, as Bazel CI has never failed with this error.

Release note: None
Release justification: Build-only change

81848: cli: fix statement-bundle recreate r=yuzefovich a=yuzefovich

In 22.1 time frame, the statement file in the bundle changed its
extension from `txt` to `sql`, so currently `statement-bundle recreate`
with 22.1 CRDB binary fails if the bundle was collected on 22.1. This
commit fixes the issue by attempting to use `sql` extension first, and
if that fails, trying `txt` extension (so that older bundles can be
recreated with a newer binary).

Release note: None

Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
@craig craig bot closed this as completed in cc71281 May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@msbutler msbutler

Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) 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.

backupccl: create readAsOfIterator for RESTORE msbutler/cockroach
1 participant
@msbutler