-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix Go routine leaks in streaming calls #15293
Fix Go routine leaks in streaming calls #15293
Conversation
…there are no go-routine leaks Signed-off-by: Manan Gupta <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Manan Gupta <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15293 +/- ##
==========================================
+ Coverage 67.41% 67.54% +0.12%
==========================================
Files 1560 1561 +1
Lines 192752 193371 +619
==========================================
+ Hits 129952 130613 +661
+ Misses 62800 62758 -42 ☔ View full report in Codecov by Sentry. |
Leaking go routines is a serious issue, and therefore the fix is being backported. |
There are other methods as well |
Signed-off-by: Manan Gupta <[email protected]>
@harshit-gangal Super catch! I've fixed them too 💯 |
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Dirkjan Bussink <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: Manan Gupta <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
Description
This PR fixes the issue described in #14201.
On following the steps listed in the issue, it was noticed that we indeed had a bunch of go-routines being spawned off, that never really went away -
Notice the count of the go-routines which are all on the same line. Its 31, but running the queries in a loop causes it to increase by 1 for each iteration.
The problem causes go-routines to leak which will eventually lead to an OOM. The fix however was pretty straightforward.
In the
StreamExecute
RPC code, we have the following commentvitess/go/vt/vttablet/grpctabletconn/conn.go
Lines 142 to 151 in f1a95e1
This however wasn't being followed in the other streaming RPCs, like
BeginStreamExecute
causing the go routines to leak.Related Issue(s)
Checklist
Deployment Notes