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.

Sign up for GitHub

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

Jump to bottom

txn: memory of previous execution can not be released when retrying a transaction #56837

Closed
AndreMouche opened this issue Oct 25, 2024 · 2 comments · Fixed by #57282
Closed

txn: memory of previous execution can not be released when retrying a transaction #56837

AndreMouche opened this issue Oct 25, 2024 · 2 comments · Fixed by #57282
Assignees
@you06
Labels
affects-5.4 This bug affects the 5.4.x(LTS) versions. 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.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/major sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.

Comments

@AndreMouche
Copy link
Contributor

Enhancement

When retrying a transaction, for example, when we meet lock-conflict error, the memory of the previous execution can not be released until the txn commits successfully. It may cause conflict txn killed by tidb_mem_quota_query or make TiDB OOM

This issue was fix in tikv/client-go#1451 since v8.4.0, while I think we need PR to fix it in lower versions.
@AndreMouche AndreMouche added type/enhancement The issue or PR belongs to an enhancement. sig/transaction SIG:Transaction labels Oct 25, 2024
@AndreMouche AndreMouche changed the title memory of previous execution can not be released when retrying a transaction txn: memory of previous execution can not be released when retrying a transaction Oct 25, 2024
@cfzjywxk
Copy link
Contributor

Related issue tikv/client-go#1375.
Some evaluations are needed for the cherrr-pick fix. If it's decided to pick the fix to release version, this issue could be marked with "bug" label for the pick.

@cfzjywxk
Copy link
Contributor

After some discussions, the issue would be fixed in the release versions, changing the label to "bug".

For versions >= v8.4, the issue is already fixed as the new ART memdb is used by default instead of the red-black tree one.

For versions < v8.4, the fix would be picked for RB-tree memdb, the solution is:

  1. When write conflict error is encounterred, the memdb.Cleanup would be called to revert the staging memory content of the current statement.
  2. Do not delete the nodes in index tree in the memdb and mark them with delete flags, as in most cases the to be written keys are the same retrying the statement. The value log is reverted like before.
  3. Reuse the nodes when retrying the statement.

@cfzjywxk cfzjywxk added type/bug The issue is confirmed as a bug. 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.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/major and removed type/enhancement The issue or PR belongs to an enhancement. labels Oct 29, 2024
@ti-chi-bot ti-chi-bot bot added may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Oct 29, 2024
@cfzjywxk cfzjywxk added affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.1 This bug affects the 6.1.x(LTS) versions. and removed may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Oct 29, 2024
@you06 you06 mentioned this issue Oct 31, 2024
Merged
@ti-chi-bot ti-chi-bot added the affects-8.5 This bug affects the 8.5.x(LTS) versions. label Nov 1, 2024
@cfzjywxk cfzjywxk removed the affects-8.5 This bug affects the 8.5.x(LTS) versions. label Nov 14, 2024
ti-chi-bot bot pushed a commit to tikv/client-go that referenced this issue Nov 14, 2024
@you06 @cfzjywxk
membuffer: fix memory leak in red-black tree (#1483)
86678db 
close #1375, ref pingcap/tidb#56837

Signed-off-by: you06 <you1474600@gmail.com>

Co-authored-by: cfzjywxk <cfzjywxk@gmail.com>
@you06 you06 mentioned this issue Nov 18, 2024
Merged
13 tasks
@ti-chi-bot ti-chi-bot bot closed this as completed in e234164 Nov 20, 2024
@ti-chi-bot ti-chi-bot mentioned this issue Nov 20, 2024
Merged
13 tasks
ti-chi-bot bot pushed a commit that referenced this issue Nov 21, 2024
@ti-chi-bot
*: bump client-go with some regression tests (#57282) (#57563)
9862e83 
ref #55287, close #56837, ref #57338, close #57425
@ti-chi-bot ti-chi-bot mentioned this issue Nov 27, 2024
Merged
13 tasks
This was referenced Nov 27, 2024
Merged
Merged
Merged
ti-chi-bot bot pushed a commit that referenced this issue Dec 5, 2024
@ti-chi-bot
*: bump client-go with some regression tests (#57282) (#57763)
858ba74 
ref #55287, close #56837, ref #57338, close #57425
ti-chi-bot bot pushed a commit that referenced this issue Dec 5, 2024
@ti-chi-bot
*: bump client-go with some regression tests (#57282) (#57761)
bc456c1 
ref #55287, close #56837, ref #57338, close #57425
ti-chi-bot bot pushed a commit that referenced this issue Dec 5, 2024
@ti-chi-bot
*: bump client-go with some regression tests (#57282) (#57765)
4b4b177 
ref #55287, close #56837, ref #57338, close #57425
ti-chi-bot bot pushed a commit that referenced this issue Dec 16, 2024
@ti-chi-bot
*: bump client-go with some regression tests (#57282) (#57764)
6722c39 
ref #55287, close #56837, ref #57338, close #57425
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@you06 you06

Labels
affects-5.4 This bug affects the 5.4.x(LTS) versions. 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.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/major sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

*: bump client-go with some regression tests you06/tidb
4 participants
@AndreMouche @cfzjywxk @you06 @ti-chi-bot