txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity #16621

MyonKeminta · 2024-03-08T11:12:24Z

What is changed and how it works?

Issue Number: Close #16620

What's Changed:

Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity.

…stic transaction's primary protected, which may break transaction atomicity Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

…k-txn-status-not-protecting-rollback-on-optimistic-primary

ti-chi-bot · 2024-03-08T11:12:26Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

cfzjywxk
ekexium

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

MyonKeminta · 2024-03-11T03:08:53Z

/release

sre-bot · 2024-03-11T03:57:27Z

download tikv at http://fileserver.pingcap.net/download/builds/pingcap/tikv/pr/bf2ec204d6b0aaf3bf9a06862ae2deded7b72fbd/centos7/tikv-server.tar.gz

…back-on-optimistic-primary

cfzjywxk

LGTM

src/storage/txn/actions/check_txn_status.rs

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta · 2024-03-11T10:07:20Z

components/resolved_ts/src/cmd.rs

+                lock_type: LockType::Put,
+            },
+            // Rollback of the txn@start_ts=7 will be missing as overlapped rollback is not
+            // hanlded.


@overvenus PTAL at changes to this test, thanks!

MyonKeminta · 2024-03-12T08:14:03Z

/merge

ti-chi-bot · 2024-03-12T08:14:05Z

@MyonKeminta: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2024-03-12T08:14:07Z

This pull request has been accepted and is ready to merge.

Commit hash: 3ca0b56

…stic transaction's primary protected, which may break transaction atomicity (tikv#16621) close tikv#16620 Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: dbsid <chenhuansheng@pingcap.com>

ti-chi-bot · 2024-03-25T05:36:34Z

In response to a cherrypick label: new pull request could not be created: failed to create pull request against tikv/tikv#release-6.5 from head ti-chi-bot:cherry-pick-16621-to-release-6.5: the GitHub API request returns a 403 error: {"message":"You have exceeded a secondary rate limit and have been temporarily blocked from content creation. Please retry your request again later. If you reach out to GitHub Support for help, please include the request ID B1F4:19253:26BF37B:3FCB805:66010D62.","documentation_url":"https://docs.github.com/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits"}

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

cfzjywxk · 2024-03-26T08:22:01Z

@MyonKeminta
We need to pick the fix to release-6.5 by hand.

MyonKeminta · 2024-03-29T03:13:15Z

/run-cherry-picker

ti-chi-bot · 2024-03-29T03:15:33Z

In response to a cherrypick label: new pull request created to branch release-6.5: #16722.

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…stic transaction's primary protected, which may break transaction atomicity (#16621) (#16722) close #16620 Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

ti-chi-bot · 2024-04-11T06:16:07Z

In response to a cherrypick label: new pull request created to branch release-7.1: #16804.

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…stic transaction's primary protected, which may break transaction atomicity (#16621) (#16804) close #16620 Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-05-07T05:49:46Z

In response to a cherrypick label: new pull request created to branch release-7.5: #16954.

…stic transaction's primary protected, which may break transaction atomicity (#16621) (#16954) close #16620 Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com> Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

MyonKeminta added 2 commits March 8, 2024 19:09

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimi…

f9e5290

…stic transaction's primary protected, which may break transaction atomicity Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

Merge branch 'master' of https://github.com/tikv/tikv into m/fix-chec…

bf2ec20

…k-txn-status-not-protecting-rollback-on-optimistic-primary

ti-chi-bot bot added release-note do-not-merge/work-in-progress size/M labels Mar 8, 2024

Merge branch 'master' into m/fix-check-txn-status-not-protecting-roll…

fa91322

…back-on-optimistic-primary

MyonKeminta marked this pull request as ready for review March 11, 2024 05:21

ti-chi-bot bot removed the do-not-merge/work-in-progress label Mar 11, 2024

MyonKeminta requested review from cfzjywxk, zyguan, you06 and ekexium March 11, 2024 05:22

cfzjywxk added the type/bugfix Type: PR - Fix a bug label Mar 11, 2024

cfzjywxk approved these changes Mar 11, 2024

View reviewed changes

src/storage/txn/actions/check_txn_status.rs Show resolved Hide resolved

ti-chi-bot bot added the status/LGT1 Status: PR - There is already 1 approval label Mar 11, 2024

Fix broken tests and update some comments

3ca0b56

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

ti-chi-bot bot added size/L and removed size/M labels Mar 11, 2024

MyonKeminta commented Mar 11, 2024

View reviewed changes

MyonKeminta mentioned this pull request Mar 12, 2024

txn: Add test case to cover the case that check_txn_status on primary lock of primary txn may affect transaction correctness pingcap/tidb#51667

Merged

13 tasks

ekexium approved these changes Mar 12, 2024

View reviewed changes

ti-chi-bot bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Mar 12, 2024

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label Mar 12, 2024

ti-chi-bot bot added this to the Pool milestone Mar 12, 2024

MyonKeminta deleted the m/fix-check-txn-status-not-protecting-rollback-on-optimistic-primary branch March 12, 2024 09:11

MyonKeminta mentioned this pull request Mar 18, 2024

update fail with begin optimistic pingcap/tidb#51855

Closed

ti-chi-bot added the needs-cherry-pick-release-6.5 label Mar 25, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Mar 25, 2024

This is an automated cherry-pick of tikv#16621

55a21bc

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

MyonKeminta removed the needs-cherry-pick-release-6.5 label Mar 29, 2024

ti-chi-bot added the needs-cherry-pick-release-6.5 label Mar 29, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Mar 29, 2024

This is an automated cherry-pick of tikv#16621

71adb9c

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Mar 29, 2024

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity (#16621) #16722

Merged

9 tasks

ti-chi-bot added the needs-cherry-pick-release-7.1 label Apr 11, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Apr 11, 2024

This is an automated cherry-pick of tikv#16621

7a36c7a

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot added the needs-cherry-pick-release-7.5 label May 7, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request May 7, 2024

This is an automated cherry-pick of tikv#16621

29a2566

close tikv#16620 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request May 7, 2024

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity (#16621) #16954

Merged

9 tasks

ti-chi-bot mentioned this pull request May 7, 2024

txn: Add test case to cover the case that check_txn_status on primary lock of primary txn may affect transaction correctness (#51667) pingcap/tidb#53059

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity #16621

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity #16621

MyonKeminta commented Mar 8, 2024

ti-chi-bot bot commented Mar 8, 2024 •

edited

MyonKeminta commented Mar 11, 2024

sre-bot commented Mar 11, 2024

cfzjywxk left a comment

MyonKeminta Mar 11, 2024

MyonKeminta commented Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

ti-chi-bot commented Mar 25, 2024

cfzjywxk commented Mar 26, 2024 •

edited

MyonKeminta commented Mar 29, 2024

ti-chi-bot commented Mar 29, 2024

ti-chi-bot commented Apr 11, 2024

ti-chi-bot commented May 7, 2024

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity #16621

txn: Fix the issue that CheckTxnStatus didn't make rollback on optimistic transaction's primary protected, which may break transaction atomicity #16621

Conversation

MyonKeminta commented Mar 8, 2024

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot bot commented Mar 8, 2024 • edited

MyonKeminta commented Mar 11, 2024

sre-bot commented Mar 11, 2024

cfzjywxk left a comment

Choose a reason for hiding this comment

MyonKeminta Mar 11, 2024

Choose a reason for hiding this comment

MyonKeminta commented Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

ti-chi-bot bot commented Mar 12, 2024

ti-chi-bot commented Mar 25, 2024

cfzjywxk commented Mar 26, 2024 • edited

MyonKeminta commented Mar 29, 2024

ti-chi-bot commented Mar 29, 2024

ti-chi-bot commented Apr 11, 2024

ti-chi-bot commented May 7, 2024

ti-chi-bot bot commented Mar 8, 2024 •

edited

cfzjywxk commented Mar 26, 2024 •

edited