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

kvserver/batcheval: ignore UseRangeTombstonesForPointDeletes knob sometimes #88468

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Sep 22, 2022

We want to ignore the point deletion knob for cases where we're deleting data from system tables watched by rangefeeds. Not doing so can lead to missed updates. This is particularly critical for cluster settings.

Fixes #87575
Fixes #87479
Fixes #88202

Release note: None

@ajwerner ajwerner requested a review from a team as a code owner September 22, 2022 15:26
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@erikgrinaker
Copy link
Contributor

I'd move this to somewhere around here:

cockroach/pkg/storage/mvcc.go

Lines 6272 to 6276 in e9d3258

// Skip intents and inline values.
if key.Timestamp.IsEmpty() {
iter.NextKey()
continue
}

…etimes

We want to ignore the point deletion knob for cases where we're deleting data
from system tables watched by rangefeeds. Not doing so can lead to missed
updates. This is particularly critical for cluster settings.

Fixes cockroachdb#87575
Fixes cockroachdb#87479

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/disallow-range-tombstones-on-watched-system-tables branch from 14418e9 to d91e4f4 Compare September 22, 2022 15:47
@ajwerner ajwerner requested a review from a team as a code owner September 22, 2022 15:47
@ajwerner
Copy link
Contributor Author

Done. mvcc.go is too big.

Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ajwerner
Copy link
Contributor Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 22, 2022

This PR was included in a batch that was canceled, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Sep 22, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment