-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-23.2: storage: assert local single delete safety #114878
Conversation
ac10970
to
ad2dd02
Compare
ad2dd02
to
e9acfd0
Compare
28a4f4d
to
ce3620d
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
…rnal This is a simple refactor of the mvccResolveWriteIntent and mvccReleaseLockInternal helpers to return an enum, rather than a bool, describing the outcome. This is preparation for a subsequent commit that will introduce an assertion whenever a SingleDelete tombstone is used to clear an intent. Epic: none Release note: none
During intent resolution, examine internal keys when intending to delete an intent using a single delete tombstone to validate that it is indeed safe within the local Engine. This is a safety measure to validate that we do not violate the invariants that make singe deletion deterministic causing replica divergence. False negatives are possible, since this safety mechanism only examines the local engine state and only the state visible to the open lock table iterator at the time of proposal evaluation. Release note: none Epic: none Informs #114492. Informs #114421.
ce3620d
to
0f14f44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
EM LGTM ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Backport 2/2 commits from #114820 on behalf of @jbowens.
/cc @cockroachdb/release
During intent resolution, examine internal keys when intending to delete an
intent using a single delete tombstone to validate that it is indeed safe
within the local Engine. This is a safety measure to validate that we do not
violate the invariants that make singe deletion deterministic and cause replica
divergence. False negatives are possible, since this safety mechanism only
examines the local engine state and only the state visible to the open lock
table iterator at the time of proposal evaluation.
Release note: none
Epic: none
Informs #114492.
Informs #114421.
Release justification: Adds assertions that protect and identify corruption.