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

Primary lock fallen back from async commit cannot be resolved #24384

Closed
sticnarf opened this issue Apr 29, 2021 · 4 comments · Fixed by #22723
Closed

Primary lock fallen back from async commit cannot be resolved #24384

sticnarf opened this issue Apr 29, 2021 · 4 comments · Fixed by #22723
Assignees
Labels
severity/critical sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.

Comments

@sticnarf
Copy link
Contributor

Bug Report

1. Minimal reproduce step (Required)

It is hard to reproduce the issue deterministically. So the reproduce steps contain technical details.

Commit an async-commit transaction with multiple keys. Prewriting the primary lock triggers a fallback (probably because TiKV executes the command too slow), while the secondary lock is prewritten successfully with the async-commit protocol. Then, the TiDB instance crashes without committing any key.

Now, if we meet the secondary lock of the above transaction, TiDB cannot resolve that lock so the statement will be blocked.

https://github.com/pingcap/tidb/blob/v5.0.1/store/tikv/lock_resolver.go#L489

Here we set current_ts to zero if the lock we first meet is an async-commit lock. Therefore, the primary lock is never rolled back in check_txn_status. And because the lock returned by check_txn_status is not an async-commit lock, the following logic does not treat this case well, the lock remains unresolved forever.

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

The locks of the transaction can be successfully resolved, so the locks will not block other readers forever.

3. What did you see instead (Required)

The secondary lock can block other readers forever.

4. What is your TiDB version? (Required)

5.0.0~5.0.1

@sticnarf sticnarf added type/bug The issue is confirmed as a bug. sig/transaction SIG:Transaction severity/critical labels Apr 29, 2021
@sticnarf sticnarf self-assigned this Apr 29, 2021
@youjiali1995
Copy link
Contributor

Remember to resolve client-c and client-java. I just realized that tispark doesn't support async-commit fallback.

/cc @leiysky @solotzg @marsishandsome

@ti-srebot
Copy link
Contributor

ti-srebot commented Apr 30, 2021

Bug

1. Root Cause Analysis (RCA) (optional)

See the issue description.

2. Symptom (optional)

See the issue description.

3. All Trigger Conditions (optional)

See the issue description.

4. Workaround (optional)

No

5. Affected versions

[v5.0.0:v5.0.1]

6. Fixed versions

v5.0.2 (estimated)

@tisonkun
Copy link
Contributor

tisonkun commented May 7, 2021

@sticnarf does client-rust suffer the same issue?

@sticnarf
Copy link
Contributor Author

@sticnarf does client-rust suffer the same issue?

client-rust does not support async commit yet. So if you mix using client-rust with a 5.0 TiDB, it cannot handle all async commit locks. But if you only use client-rust, the problem does not exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/critical sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.
Projects
None yet
4 participants