koordlet: improve koordlet profiling and logs #1180
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #1180 +/- ##
=======================================
Coverage 65.72% 65.73%
=======================================
Files 290 290
Lines 30646 30648 +2
=======================================
+ Hits 20143 20146 +3
Misses 9023 9023
+ Partials 1480 1479 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
/cc @zwzhang0107 @saintube Take a look. |
cmd/koordlet/options/options.go
Outdated
| ) | ||
|
|
||
| func installHandlerForPProf(mux *http.ServeMux) { |
There was a problem hiding this comment.
why no longer use _ "net/http/pprof"
There was a problem hiding this comment.
@saintube profiling server add the timeout time to protect the service from attack.
const (
// ReadHeaderTimeout is the amount of time allowed to read
// request headers.
// HTTP timeouts are necessary to expire inactive connections
// and failing to do so might make the application vulnerable
// to attacks like slowloris which work by sending data very slow,
// which in case of no timeout will keep the connection active
// eventually leading to a denial-of-service (DoS) attack.
// References:
// - https://en.wikipedia.org/wiki/Slowloris_(computer_security)
ReadHeaderTimeout = 32 * time.Second
)
There was a problem hiding this comment.
Would you mind keeping the default registration of the pprof? We may not need to register the handlers manually. The timeout time can still be set by creating a server with http.DefaultServeMux.
There was a problem hiding this comment.
Would you mind keeping the default registration of the pprof? We may not need to register the handlers manually. The timeout time can still be set by creating a server with
http.DefaultServeMux.
@saintube Custom profliing server has been removed. If that's all right, need to /approve:)
cmd/koordlet/options/options.go
Outdated
| ServerAddr = flag.String("addr", ":9316", "port of koordlet server") | ||
| EnablePprof = flag.Bool("enable-pprof", false, "Enable pprof for koordlet.") | ||
| PprofAddr = flag.String("pprof-addr", ":9317", "The address the pprof binds to.") | ||
| KubeAPIQPS = flag.Float64("kube-api-qps", 40.0, "QPS to use while talking with kube-apiserver.") |
There was a problem hiding this comment.
how about keeping the default QPS and Burst the same as the previous config (20, 30)
There was a problem hiding this comment.
@saintube QPS and burst have been set to default values.
|
/cc @saintube Please take a look again:) |
Signed-off-by: xigang <wangxigang2014@gmail.com>
|
/lgtm |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hormes, jasonliu747, zwzhang0107 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/area koordlet |
Ⅰ. Describe what this PR does
koordlet: improve koordlet profiling and logs.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
@eahydra @jasonliu747
V. Checklist
make test