Closed JesseStutler closed 3 weeks ago
/cc @hwdef @Monokaix @lowang-bh Please take a look~ I will push the other relating pr lately.
Should also add that queue's pg statistics are still maintained in queue cache because these data is needed when close a queue.
Should also add that queue's pg statistics are still maintained in queue cache because these data is needed when close a queue.
I added a notice item to record this, please review it again.
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: Monokaix
The full list of commands accepted by this bot can be found here.
The pull request process is described here
fix #3597
Backgrounds
Each time when podgroups states changed, the controller will update the statistics of podgroup of each state in the queue's status. And at the end of each scheduling session, the volcano scheduler will also update the allocated filed in queue's status to recored the amount of the amount of resources allocated. Both components use
UpdateStatus
api to update the queue status, which will cause conflict errors. When the controller encounter such an error, it will triggerAddRateLimited
to push back the podgroup into work queue, resulting in accumulation of memory leak. See in issue #3597.Alternative
Currently the statistics of podgroup of eatch state are only used for display by vcctl, there is no need to be persisted in queue's status. So when users need to use
vcctl queue get -n [name]
orvcctl list
to display queues and each state of podgroups in queue, vcctl should calculate podgroup statistics in client side and then display them. And we can export these statistics of podgroups in each state as metrics.Implementation
syncQueue
of queue controller, we should not stat counts of podgroups in each state here, instead these statistics should recorded as metrics and then be exported outside: https://github.com/volcano-sh/volcano/blob/release-1.10/pkg/controllers/queue/queue_controller_action.go#L41-L61. AndUpdateStatus
should not be used here: https://github.com/volcano-sh/volcano/blob/release-1.10/pkg/controllers/queue/queue_controller_action.go#L84-L87, theUpdateStatus
interface will verify the resourceVersion in the apiserver, which may cause concurrent update conflicts. Instead,ApplyStatus
should be used here to avoid this situation, because we only need to update the status of the queue. It should be noted that the controller currently does not have patch queue status permissions, so we should add a patch queue/status permission to the clusterrole.vcctl get -n [name]
andvcctl list
display the statistics of podgroups in each state from queue's status directly, instead we should do one more step, query the podgroups owend in the queue, stat the counts of podgroups in each state at thevcctl
side, and then display them.