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

sql: use read ts as batch ts to send index backfill SSTs #64023

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

dt
Copy link
Member

@dt dt commented Apr 21, 2021

This changes SSTs sent by index backfills to be sent with a batch
timestamp equal to the timestamp that was used to read the rows from
which the index entries in that SST were generated.

This is done to ensure that if the index span has already GCed any
keys related to more recent SQL writes the SST will be rejected. Adding
the SST generated from older writes could otherwise accidentally re-add
an entry after a later SQL delete's tombstone had been GC'ed, as described
in #64014.

Release note (bug fix): fix a theoretical issue in index backfills that could result in stale entries that would likely fail validation (#64014).

@dt dt requested review from pbardea and ajwerner April 21, 2021 22:14
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@dt dt force-pushed the index-sst-batch-ts branch 3 times, most recently from 8c636e6 to 68e92b5 Compare April 22, 2021 03:15
Copy link
Contributor

@ajwerner ajwerner 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 7 of 7 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @pbardea)

@dt
Copy link
Member Author

dt commented Apr 22, 2021

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Apr 22, 2021

Build failed (retrying...):

This changes SSTs sent by index backfills to be sent with a batch
timestamp equal to the timestamp that was used to read the rows from
which the index entries in that SST were generated.

This is done to ensure that if the index span has already GCed any
keys related to more recent SQL writes the SST will be rejected. Adding
the SST generated from older writes could otherwise accidentally re-add
an entry after a later SQL delete's tombstone had been GC'ed, as described
in cockroachdb#64014.

Release note (bug fix): fix a theoretical issue in index backfills that could result in stale entries that would likely fail validation (cockroachdb#64014).
@dt dt requested a review from a team April 22, 2021 04:43
@craig
Copy link
Contributor

craig bot commented Apr 22, 2021

Canceled.

@dt
Copy link
Member Author

dt commented Apr 22, 2021

bors r+

@craig
Copy link
Contributor

craig bot commented Apr 22, 2021

Build succeeded:

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