fix PodGroup being incorrectly deleted due to frequent creation and deletion of pods #3375
Conversation
volcano-sh-bot
added
the
size/S
|
Can you briefly describe the processing time sequence of this scenario using a diagram? |
pkg/scheduler/cache/cache.go
Outdated
| oldPgVersion := oldJob.PgUID | ||
| newPgVersion := job.PgUID | ||
| klog.V(5).Infof("just add pguid:%v, try to delete pguid:%v", oldPgVersion, newPgVersion) | ||
| if oldPgVersion == newPgVersion { |
There was a problem hiding this comment.
In which case podgroup ID will change? In my opinion, the created podgroup will keep exist and UID is same, has nothing to do with pod creating or deleting in same job.
There was a problem hiding this comment.
The UID of the podgroup stored as JobInfo is the same as the name of the pg, and it remains unchanged. However, when the number of pods becomes 0, it triggers the deletion of the pg. When the number of pods is non-zero, a new pg will be created again. At this point, the UID of the pg itself changes, making it unique. For a detailed analysis, please refer to the issue #3374. I have provided a detailed description of the problem scenario there.
Added to the issue #3374 |
pkg/scheduler/cache/cache.go
Outdated
| metrics.DeleteJobMetrics(job.Name, string(job.Queue), job.Namespace) | ||
| klog.V(3).Infof("Job <%v:%v/%v> was deleted.", job.UID, job.Namespace, job.Name) | ||
| sc.DeletedJobs.Forget(obj) | ||
| if oldJob, found := sc.Jobs[job.UID]; found { |
There was a problem hiding this comment.
change to:
oldJob, found := sc.Jobs[job.UID];
if !found {
return
}
guoqinwill
force-pushed
the
fix-podgroup
branch
from
59f211d
to
63dadba
Compare
| metrics.DeleteJobMetrics(job.Name, string(job.Queue), job.Namespace) | ||
| klog.V(3).Infof("Job <%v:%v/%v> was deleted.", job.UID, job.Namespace, job.Name) | ||
| sc.DeletedJobs.Forget(obj) | ||
| } |
There was a problem hiding this comment.
If oldPgVersion is not equal to newPgVersion, the current obj should also be discarded. Otherwise, DeletedJobs will be retried infinitely. In addition, DeletedJobs may be full after a long time.
There was a problem hiding this comment.
Forget and Done method can guarantee old key be discarded.
| oldJob, found := sc.Jobs[job.UID] | ||
| if !found { | ||
| klog.V(3).Infof("Failed to find Job <%v:%v/%v>, ignore it", job.UID, job.Namespace, job.Name) | ||
| return |
There was a problem hiding this comment.
Should obj be discarded here to avoid infinite retries?
Signed-off-by: guoqin <gq411will@163.com> Signed-off-by: g00673948 <guoqin10@huawei.com>
guoqinwill
force-pushed
the
fix-podgroup
branch
from
63dadba
to
2832dfe
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: william-wang 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 |
volcano-sh-bot
added
the
approved
|
/lgtm |
fix :#3374