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

fix goroutine leak when tls enabled #3081

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

DanielZhangQD
Copy link
Contributor

@DanielZhangQD DanielZhangQD commented Aug 6, 2020

What problem does this PR solve?

Fix #3079

What is changed and how does it work?

Disable keepalive for TLS connections, more detail refer to elastic/cloud-on-k8s#854 (comment)

Check List

Tests

  • Unit test
  • E2E test
  • Stability test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has Go code change

Related changes

  • Need to cherry-pick to the release branch

Does this PR introduce a user-facing change?:

Fix goroutine leak when TLS enabled

@DanielZhangQD
Copy link
Contributor Author

DanielZhangQD commented Aug 6, 2020

Deploy 4 TiDB Clusters with TLS enabled and 4 TiDB Clusters with TLS disabled, after 12 hours running with this fix:
The overall profile:

Count Profile
15699 allocs
0 block
0 cmdline
291 goroutine
15699 heap
0 mutex
0 profile
34 threadcreate
0 trace

The memory usage is less than 60 MB and the heap profile as below:

Fetching profile over HTTP from http://10.233.119.199:6060/debug/pprof/heap
Saved profile in /root/pprof/pprof.tidb-controller-manager.alloc_objects.alloc_space.inuse_objects.inuse_space.022.pb.gz
File: tidb-controller-manager
Type: inuse_space
Time: Aug 5, 2020 at 8:28pm (EDT)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 21170kB, 85.44% of 24776.52kB total
Showing top 10 nodes out of 182
      flat  flat%   sum%        cum   cum%
 4440.76kB 17.92% 17.92%  4440.76kB 17.92%  runtime/pprof.writeHeapInternal
 3632.60kB 14.66% 32.58%  3632.60kB 14.66%  reflect.unsafe_NewArray
 3073.03kB 12.40% 44.99%  3073.03kB 12.40%  go.uber.org/zap/zapcore.newCounters
 2560.31kB 10.33% 55.32%  2560.31kB 10.33%  github.com/modern-go/reflect2.newUnsafeStructField
 2049.08kB  8.27% 63.59%  2049.08kB  8.27%  reflect.mapassign
 1536.05kB  6.20% 69.79%  1536.05kB  6.20%  github.com/json-iterator/go.(*Iterator).ReadString
 1301.24kB  5.25% 75.04%  1301.24kB  5.25%  golang.org/x/net/http2.(*ClientConn).frameScratchBuffer
 1024.57kB  4.14% 79.18%  1024.57kB  4.14%  encoding/json.typeFields
 1024.19kB  4.13% 83.31%  1024.19kB  4.13%  github.com/aws/aws-sdk-go/aws/endpoints.init
  528.17kB  2.13% 85.44%   528.17kB  2.13%  k8s.io/apimachinery/pkg/watch.(*Broadcaster).Watch.func1

@codecov-commenter
Copy link

Codecov Report

Merging #3081 into master will decrease coverage by 0.01%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master    #3081      +/-   ##
==========================================
- Coverage   42.34%   42.33%   -0.02%     
==========================================
  Files         156      156              
  Lines       16763    16767       +4     
==========================================
  Hits         7099     7099              
- Misses       9091     9094       +3     
- Partials      573      574       +1     
Flag Coverage Δ
#unittest 42.33% <87.50%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@weekface weekface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DanielZhangQD DanielZhangQD merged commit eb30500 into pingcap:master Aug 6, 2020
@DanielZhangQD DanielZhangQD deleted the mem0 branch August 6, 2020 03:30
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Aug 6, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #3083

ti-srebot added a commit that referenced this pull request Aug 6, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiDB OOM when TLS is enabled
5 participants