yangwwei commented on a change in pull request #175:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/175#discussion_r444593859
##########
File path: pkg/scheduler/policies/sorting_policy.go
##########
@@ -26,14 +26,15 @@ import (
type SortPolicy int
const (
- FifoSortPolicy SortPolicy = iota // first in first out, submit time
+ Undefined SortPolicy = iota // not initialised or parsing failed
+ FifoSortPolicy // first in first out, submit time
FairSortPolicy // fair based on usage
StateAwarePolicy // only 1 app in starting state
- Undefined // not initialised or parsing failed
+ PriorityPolicy // sort on the priority
Review comment:
this could be confusing. why we are sharing the `SortPolicy` type
between apps and requests?
##########
File path: pkg/scheduler/scheduling_allocation_ask.go
##########
@@ -108,9 +108,15 @@ func (saa *schedulingAllocationAsk) getCreateTime()
time.Time {
return saa.createTime
}
-// Normalised priority
-// Currently a direct conversion.
+// Normalised priority, a direct conversion with a minimal value of 0.
+// 0 is the lowest priority
+// MaxInt the highest.
+// No support for named priorities
func (saa *schedulingAllocationAsk) normalizePriority(priority *si.Priority)
int32 {
// TODO, really normalize priority from ask
- return priority.GetPriorityValue()
+ prioVal := priority.GetPriorityValue()
+ if prioVal < 0 {
+ return 0
+ }
Review comment:
why we have this assumption that priority must be >= 0?
I checked some doc such as:
https://kubernetes.io/blog/2019/04/16/pod-priority-and-preemption-in-kubernetes/
> If you give a negative priority to your non-critical workload, Cluster
Autoscaler does not add more nodes to your cluster when the non-critical pods
are pending. Therefore, you won’t incur higher expenses.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
