You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/12/13 15:36:13 UTC
[GitHub] [yunikorn-core] craigcondit commented on a diff in pull request #476: [YUNIKORN-1464] Add new queue configuration options
craigcondit commented on code in PR #476:
URL: https://github.com/apache/yunikorn-core/pull/476#discussion_r1047323691
##########
pkg/scheduler/objects/queue.go:
##########
@@ -296,39 +297,64 @@ func (sq *Queue) setTemplate(conf configs.ChildTemplate) error {
return nil
}
-// UpdateSortType updates the sortType for the queue based on the current properties.
-func (sq *Queue) UpdateSortType() {
+// UpdateQueueProperties updates the queue properties defined as text
+func (sq *Queue) UpdateQueueProperties() {
sq.Lock()
defer sq.Unlock()
// set the defaults, override with what is in the configured properties
if sq.isLeaf {
// walk over all properties and process
var err error
- sq.sortType = policies.Undefined
for key, value := range sq.properties {
- if key == configs.ApplicationSortPolicy {
+ switch key {
+ case configs.ApplicationSortPolicy:
sq.sortType, err = policies.SortPolicyFromString(value)
if err != nil {
log.Logger().Debug("application sort property configuration error",
zap.Error(err))
}
- } else {
+ // if it is not defined default to fifo
+ if sq.sortType == policies.Undefined {
+ sq.sortType = policies.FifoSortPolicy
+ }
+ case configs.PreemptionPolicy:
+ sq.preemptionFenced, err = isFencingEnabled(value)
+ if err != nil {
+ log.Logger().Debug("queue fencing property configuration error",
+ zap.Error(err))
+ }
+ case configs.PreemptionDelay:
+ sq.preemptionDelay, err = time.ParseDuration(value)
+ if err != nil {
+ log.Logger().Debug("preemption delay property configuration error",
+ zap.Error(err))
+ }
+ default:
// skip unknown properties just log them
log.Logger().Debug("queue property skipped",
zap.String("key", key),
zap.String("value", value))
}
}
- // if it is not defined default to fifo
- if sq.sortType == policies.Undefined {
- sq.sortType = policies.FifoSortPolicy
- }
+
return
}
// set the sorting type for parent queues
sq.sortType = policies.FairSortPolicy
}
+func isFencingEnabled(str string) (bool, error) {
+ switch str {
+ // fifo is the default policy when not set
+ case "default":
+ return false, nil
+ case "fence":
+ return true, nil
+ default:
Review Comment:
We need to also add "disabled" to this as that's going to be how we disable preemption for a queue. Also, we should probably have these be constants.
##########
pkg/scheduler/objects/queue.go:
##########
@@ -296,39 +297,64 @@ func (sq *Queue) setTemplate(conf configs.ChildTemplate) error {
return nil
}
-// UpdateSortType updates the sortType for the queue based on the current properties.
-func (sq *Queue) UpdateSortType() {
+// UpdateQueueProperties updates the queue properties defined as text
+func (sq *Queue) UpdateQueueProperties() {
sq.Lock()
defer sq.Unlock()
// set the defaults, override with what is in the configured properties
if sq.isLeaf {
// walk over all properties and process
var err error
- sq.sortType = policies.Undefined
for key, value := range sq.properties {
- if key == configs.ApplicationSortPolicy {
+ switch key {
+ case configs.ApplicationSortPolicy:
sq.sortType, err = policies.SortPolicyFromString(value)
if err != nil {
log.Logger().Debug("application sort property configuration error",
zap.Error(err))
}
- } else {
+ // if it is not defined default to fifo
+ if sq.sortType == policies.Undefined {
+ sq.sortType = policies.FifoSortPolicy
+ }
+ case configs.PreemptionPolicy:
+ sq.preemptionFenced, err = isFencingEnabled(value)
+ if err != nil {
+ log.Logger().Debug("queue fencing property configuration error",
+ zap.Error(err))
+ }
+ case configs.PreemptionDelay:
+ sq.preemptionDelay, err = time.ParseDuration(value)
+ if err != nil {
+ log.Logger().Debug("preemption delay property configuration error",
+ zap.Error(err))
+ }
Review Comment:
We need to ensure that preemptionDelay is positive, and default to 30s if not specified.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org