scheduler: improve gang log #2009
Conversation
Signed-off-by: googs1025 <googs1025@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2009 +/- ##
==========================================
+ Coverage 68.65% 68.67% +0.01%
==========================================
Files 421 421
Lines 38916 38916
==========================================
+ Hits 26719 26726 +7
+ Misses 9880 9872 -8
- Partials 2317 2318 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Why do we need to downgrade error logs to V4? |
Indeed, this level needs to be confirmed. However, setting the log level can effectively reduce the number of log prints. |
| @@ -145,7 +146,7 @@ func (gang *Gang) tryInitByPodConfig(pod *v1.Pod, args *config.CoschedulingArgs) | |||
| matchPolicy := util.GetGangMatchPolicyByPod(pod) | |||
| if matchPolicy != extension.GangMatchPolicyOnlyWaiting && matchPolicy != extension.GangMatchPolicyWaitingAndRunning && | |||
| matchPolicy != extension.GangMatchPolicyOnceSatisfied { | |||
| klog.Errorf("pod's annotation AnnotationGangMatchPolicy illegal, gangName: %v, value: %v", | |||
| klog.V(4).Infof("pod's annotation AnnotationGangMatchPolicy illegal, gangName: %v, value: %v", | |||
There was a problem hiding this comment.
We should determine if matchPolicy is an empty string and if not, log an error.
| totalChildrenNum, err := strconv.ParseInt(pod.Annotations[extension.AnnotationGangTotalNum], 10, 32) | ||
| if err != nil { | ||
| klog.Errorf("pod's annotation totalNumber illegal, gangName: %v, value: %v", | ||
| klog.V(4).ErrorS(err, "pod's annotation totalNumber illegal, gangName: %v, value: %v", | ||
| gang.Name, pod.Annotations[extension.AnnotationGangTotalNum]) | ||
| totalChildrenNum = int64(minRequiredNumber) | ||
| } else if totalChildrenNum != 0 && totalChildrenNum < int64(minRequiredNumber) { | ||
| klog.Errorf("pod's annotation totalNumber cannot less than minRequiredNumber, gangName: %v, totalNumber: %v,minRequiredNumber: %v", | ||
|
|
||
| klog.V(4).Infof("pod's annotation totalNumber cannot less than minRequiredNumber, gangName: %v, totalNumber: %v,minRequiredNumber: %v", | ||
| gang.Name, pod.Annotations[extension.AnnotationGangTotalNum], minRequiredNumber) | ||
| totalChildrenNum = int64(minRequiredNumber) | ||
| } | ||
| gang.TotalChildrenNum = int(totalChildrenNum) |
There was a problem hiding this comment.
We encourage cleaner code, which of course includes improved logging.
| totalChildrenNum, err := strconv.ParseInt(pod.Annotations[extension.AnnotationGangTotalNum], 10, 32) | |
| if err != nil { | |
| klog.Errorf("pod's annotation totalNumber illegal, gangName: %v, value: %v", | |
| klog.V(4).ErrorS(err, "pod's annotation totalNumber illegal, gangName: %v, value: %v", | |
| gang.Name, pod.Annotations[extension.AnnotationGangTotalNum]) | |
| totalChildrenNum = int64(minRequiredNumber) | |
| } else if totalChildrenNum != 0 && totalChildrenNum < int64(minRequiredNumber) { | |
| klog.Errorf("pod's annotation totalNumber cannot less than minRequiredNumber, gangName: %v, totalNumber: %v,minRequiredNumber: %v", | |
| klog.V(4).Infof("pod's annotation totalNumber cannot less than minRequiredNumber, gangName: %v, totalNumber: %v,minRequiredNumber: %v", | |
| gang.Name, pod.Annotations[extension.AnnotationGangTotalNum], minRequiredNumber) | |
| totalChildrenNum = int64(minRequiredNumber) | |
| } | |
| gang.TotalChildrenNum = int(totalChildrenNum) | |
| var expectedTotalNum int64 | |
| s := pod.Annotations[extension.AnnotationGangTotalNum] | |
| if s != "" { | |
| var err error | |
| expectedTotalNum, err = strconv.ParseInt(s) | |
| if err != nil { | |
| klog.V(4).ErrorS(err, "illegal annotation", "key", extension.AnnotationGangTotalNum, "value": s) | |
| } | |
| } | |
| if expectedTotalNum < int64(minRequiredNumber) { | |
| expectedTotalNum = int64(minRequiredNumber) | |
| } | |
| gang.TotalChildrenNum = int(expectedTotalNum) |
| @@ -157,15 +158,15 @@ func (gang *Gang) tryInitByPodConfig(pod *v1.Pod, args *config.CoschedulingArgs) | |||
|
|
|||
| waitTime, err := time.ParseDuration(pod.Annotations[extension.AnnotationGangWaitTime]) | |||
| if err != nil || waitTime <= 0 { | |||
| klog.Errorf("pod's annotation GangWaitTimeAnnotation illegal, gangName: %v, value: %v", | |||
| klog.V(4).ErrorS(err, "pod's annotation GangWaitTimeAnnotation illegal, gangName: %v, value: %v", | |||
There was a problem hiding this comment.
All these logs have same problems, should improve these codes to make more clean.
|
/assign @ZiMengSheng |
Ⅰ. Describe what this PR does
I modified the current log level output method. In order to reduce log printing, I only print when --v=4 is used, and use klog.V().ErrorS for the err!=nil part.
Ⅱ. Does this pull request fix one issue?
fix: #1895 (comment)
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
V. Checklist
make test