-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
store/tikv: Fix goroutine leak in tikv client #20808
store/tikv: Fix goroutine leak in tikv client #20808
Conversation
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@cfzjywxk @tiancaiamao PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
/merge |
/run-all-tests |
@MyonKeminta merge failed. |
/merge |
Your auto merge job has been accepted, waiting for:
|
/merge |
Your auto merge job has been accepted, waiting for:
|
/run-all-tests |
cherry pick to release-3.0 in PR #20863 |
cherry pick to release-4.0 failed |
/run-cherry-picker |
cherry pick to release-4.0 in PR #20870 |
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: ti-srebot <ti-srebot@pingcap.com> cherry pick pingcap#20808 to release-3.0 Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: MyonKeminta MyonKeminta@users.noreply.github.com
What problem does this PR solve?
Issue Number: close #20654
Problem Summary: There's a goroutine leak found in tikv client, and it's confirmed to be caused by forgetting closing the etcd client in
EtcdSafePointKV
.The bug doesn't harm TiDB, since the
tikvStore
longs for the whole lifetime of the TiDB process. However, it affects other projects that make use of TiKV client. For example, CDC may open and close tikv client many times.What is changed and how it works?
This PR fixes the goroutine leak by adding a
Close
interface to SafePointKV, and invoke it inClose
method oftikvStore
.Related changes
The problem should also exists in release 2.x, but I'm not sure if it's necessary. Nor do I know if we still have release plan of 2.1.x.
Check List
Tests
The code segment of the test program:
Run and type
100
(create and close tikv client 100 times), dump the goroutines by pprof after the log stopps:After the fix and run again:
Side effects
-
Release note