Fix leaking goroutines in QuotaEvaluator #110246
Conversation
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wojtek-t 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 |
k8s-ci-robot
added
approved
| // and often not started for some of those (e.g. aggregated apiserver) | ||
| // we explicitly shut it down on stopCh signal even if it wasn't | ||
| // effectively started. | ||
| go evaluator.shutdownOnStop() |
There was a problem hiding this comment.
does it means we start only once but we shutdown multiple times?
There was a problem hiding this comment.
Sorry - maybe I wasn't clear.
What I meant is that:
- we create a separate QuotaEvaluator for every layer (separate for kube-apiserver, separate for agregated, separate for apiextensions)
- the creation of it actually creates a queue which starts a goroutine that is updating metrics (this is created in line 125 so once per quota evaluator)
- the newly added shutdownOnStop is also created here so called exactly once (per QuotaEvaluator)
- the start() is called at most once - line 606
There was a problem hiding this comment.
and why is not valid to keep it as it was?
There was a problem hiding this comment.
Not sure I understand the question.
As I wrote above - when I create QuotaEvaluator, this is underneath starting a gorotuine (created by workqueue.NewNamed).
That goroutine is shut down only when run() (renamed to start()) method is called.
run() is called on the first Evaluate - if that is not called - the goroutine will be leaked.
There was a problem hiding this comment.
ahhhhhhhhhhhhhhhhhhhhhhhh ... sorry friday and lack coffeeeeeee
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
Ref #108483
/kind cleanup
/priority important-longterm
/sig api-machinery