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

GC use BatchResolveLocks may miss some pessimistic locks in rare cases #45134

Closed
MyonKeminta opened this issue Jul 3, 2023 · 0 comments · Fixed by #45143
Closed

GC use BatchResolveLocks may miss some pessimistic locks in rare cases #45134

MyonKeminta opened this issue Jul 3, 2023 · 0 comments · Fixed by #45143
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 severity/major sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.

Comments

@MyonKeminta
Copy link
Contributor

MyonKeminta commented Jul 3, 2023

Bug Report

Please answer these questions before submitting your issue. Thanks!

This problem is introduced when fixing #43243 .

When BatchResolveLocks meets a pessimistic lock, it calls resolvePessimisticLock immediately, before calling getTxnStatus, considering that we will delete pessimistic locks no matter what state it's belonging transactions are:

https://github.com/tikv/client-go/blob/fbec0230608310ecd18caaea76c0cdbb23cc933a/txnkv/txnlock/lock_resolver.go#L248-L266

However, it's not noticed that resolvePessimisticLocks ignores primary locks (to be more precise, locks whose primary field points to itself), considering that CheckTxnStatus must has been called on it:

https://github.com/tikv/client-go/blob/fbec0230608310ecd18caaea76c0cdbb23cc933a/txnkv/txnlock/lock_resolver.go#L1178-L1181

This may cause that in some rare cases, some pessimistic locks may be left after GCing.

The problem usually happen when node crashing, RPC failing, etc. so that transactions are leaving uncleared locks, which are not common cases. Once it happens, it will possibly affect the lagging of CDC and stale read.

@MyonKeminta MyonKeminta added type/bug The issue is confirmed as a bug. severity/major affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 labels Jul 3, 2023
@ti-chi-bot ti-chi-bot bot added 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. labels Jul 3, 2023
@MyonKeminta MyonKeminta removed 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. labels Jul 3, 2023
ti-chi-bot bot pushed a commit that referenced this issue Jul 4, 2023
ti-chi-bot bot pushed a commit that referenced this issue Jul 4, 2023
@Ivy-YinSu Ivy-YinSu added the sig/transaction SIG:Transaction label Jul 7, 2023
ti-chi-bot bot pushed a commit that referenced this issue Jul 12, 2023
MyonKeminta added a commit to ti-chi-bot/tidb that referenced this issue Aug 8, 2023
ti-chi-bot bot pushed a commit that referenced this issue Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 severity/major sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.
Projects
None yet
2 participants