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

Async commit can mistakenly return success, while it's actually rolled back #33641

Closed
ekexium opened this issue Mar 31, 2022 · 1 comment · Fixed by tikv/client-go#492
Closed
Labels
affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 severity/major sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.

Comments

@ekexium
Copy link
Member

ekexium commented Mar 31, 2022

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

It's a bit hard to reproduce.
First, use a modified client-go and set its maxCommitTs to be 40s.
Then run the test with failpoint. pd-ctl and tikv-ctl are needed.
https://gist.github.com/ekexium/5152b02ba7a1a63290c1c7539a7d047f

2. What did you expect to see? (Required)

Results of commit statements reflect their real status.

3. What did you see instead (Required)

Both transactions return commit success to the client.
While from the logs and MVCC records we can see the pessimistic transaction is rolled back, and it failed in its commit phase.
Locked keys (the row key, in this example) were rolled back, while non-unique index keys commit succeeded. It breaks atomicity and may risk data-index consistency.

4. What is your TiDB version? (Required)

master.

@ekexium ekexium added the type/bug The issue is confirmed as a bug. label Mar 31, 2022
@ti-chi-bot ti-chi-bot added may-affects-4.0 This bug maybe affects 4.0.x versions. may-affects-5.0 This bug maybe affects 5.0.x versions. may-affects-5.1 This bug maybe affects 5.1.x versions. may-affects-5.2 This bug maybe affects 5.2.x versions. may-affects-5.3 This bug maybe affects 5.3.x versions. may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.0 labels Apr 1, 2022
@ekexium ekexium added affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 and removed may-affects-4.0 This bug maybe affects 4.0.x versions. labels Apr 1, 2022
@ti-chi-bot ti-chi-bot removed may-affects-5.1 This bug maybe affects 5.1.x versions. may-affects-5.2 This bug maybe affects 5.2.x versions. may-affects-5.3 This bug maybe affects 5.3.x versions. may-affects-6.0 may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-5.0 This bug maybe affects 5.0.x versions. labels Apr 1, 2022
@ChenPeng2013 ChenPeng2013 added the sig/transaction SIG:Transaction label Apr 1, 2022
@ekexium ekexium reopened this May 13, 2022
@ekexium
Copy link
Member Author

ekexium commented May 16, 2022

Fixed on master by 4b00ee2 that updates client-go.

@ekexium ekexium closed this as completed May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 severity/major sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.
Projects
None yet
4 participants