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

tikv: fix primary selection when delete-your-writes (#18244) #18249

Merged
merged 3 commits into from
Jun 29, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #18244 to release-3.1


What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

  1. Push down condition check to TiKV for prewrite request #9127 introduce addition useless but sometime wrong DEL mutation for delete-your-writes
  2. store: check constraint for "Delete-Your-Writes" records when txn commit #14968 try to remove wrong DEL mutation
  3. but wrong DEL mutation can take role as 2pc primary key(or transaction record) to works well in some situation, but store: check constraint for "Delete-Your-Writes" records when txn commit #14968 remove it will cause prewrite locks's primary property point to a unwriten primary key
  4. any read-write request see those uncommited lock with wrong primary key value will treat them belong rollbacked transaction, but real transaction invoker maybe think transaction has be success.

so it will cause read/write inconsistent result when meet lock that point to a primary key has be insert/delete in own txn.

https://github.com/pingcap/tidb/pull/18244/files#diff-d11e207d2529a00e79d72b84ae5172caR722 will fail and read nothing to reproduce it in 2pc test case level.

What is changed and how it works?

What's Changed, How it Works:

choose first key didn't be delete by itself as primary key.

Related changes

  • Need to cherry-pick to the release branch 3.0

Check List

Tests

  • Unit test

Side effects

  • n/a

Release note

  • Fix read/write inconsistent result when meet lock that point to a primary key has be insert/delete in own txn

This change is Reviewable

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot added sig/transaction SIG:Transaction priority/release-blocker This issue blocks a release. Please solve it ASAP. type/3.1-cherry-pick type/bugfix This PR fixes a bug. labels Jun 29, 2020
@ti-srebot ti-srebot added this to the v3.1.3 milestone Jun 29, 2020
@lysu
Copy link
Contributor

lysu commented Jun 29, 2020

/run-all-tests

Copy link
Contributor

@youjiali1995 youjiali1995 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor Author

@youjiali1995,Thanks for you review.

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor Author

@jackysp,Thanks for you review.

jackysp
jackysp previously approved these changes Jun 29, 2020
@ti-srebot
Copy link
Contributor Author

@jackysp, Thanks for your review, however we are sorry that your vote won't be count. You already give a LGTM to this PR

@jackysp
Copy link
Member

jackysp commented Jun 29, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 29, 2020
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@coocood
Copy link
Member

coocood commented Jun 29, 2020

The failure is not random.

@ti-srebot
Copy link
Contributor Author

@jackysp, Thanks for your review, however we are sorry that your vote won't be count. You already give a LGTM to this PR

@jackysp
Copy link
Member

jackysp commented Jun 29, 2020

/merge

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 55fa027 into pingcap:release-3.1 Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/release-blocker This issue blocks a release. Please solve it ASAP. sig/transaction SIG:Transaction status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug. type/3.1-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants