pbacsko commented on code in PR #1034:
URL: https://github.com/apache/yunikorn-core/pull/1034#discussion_r2477852722
##########
pkg/scheduler/objects/queue.go:
##########
@@ -372,17 +373,67 @@ func (sq *Queue) applyConf(conf configs.QueueConfig,
silence bool) error {
// Load the max & guaranteed resources and maxApps for all but the root
queue
if sq.Name != configs.RootQueue {
+ oldMaxResource := sq.maxResource
if err = sq.setResourcesFromConf(conf.Resources); err != nil {
return err
}
sq.maxRunningApps = conf.MaxApplications
sq.updateMaxRunningAppsMetrics()
+ sq.setPreemptionSettings(oldMaxResource, conf)
}
sq.properties = conf.Properties
return nil
}
+// setPreemptionSettings Set Quota change preemption settings
+func (sq *Queue) setPreemptionSettings(oldMaxResource *resources.Resource,
conf configs.QueueConfig) {
+ newMaxResource, err := resources.NewResourceFromConf(conf.Resources.Max)
+ if err != nil {
+ log.Log(log.SchedQueue).Error("parsing failed on max resources
this should not happen",
+ zap.String("queue", sq.QueuePath),
+ zap.Error(err))
+ return
+ }
+ reset := false
+ set := false
+ switch {
+ // Set max res earlier but not now
+ case resources.IsZero(newMaxResource) &&
!resources.IsZero(oldMaxResource):
+ reset = true
+ // Set max res now but not earlier
+ case !resources.IsZero(newMaxResource) &&
resources.IsZero(oldMaxResource) && conf.Preemption.Delay != 0:
+ set = true
+ // Set max res earlier and now as well
+ default:
+ switch {
+ // Quota decrease
+ case resources.StrictlyGreaterThan(oldMaxResource,
newMaxResource) && conf.Preemption.Delay != 0:
+ set = true
+ // Quota increase
+ case resources.StrictlyGreaterThan(newMaxResource,
oldMaxResource) && conf.Preemption.Delay != 0:
+ reset = true
+ // Quota remains as is but delay has changed
+ case resources.Equals(oldMaxResource, newMaxResource) &&
conf.Preemption.Delay != 0 && sq.quotaChangePreemptionDelay !=
conf.Preemption.Delay:
+ sq.quotaChangePreemptionDelay = conf.Preemption.Delay
+ default:
+ // noop
+ }
+ }
+
+ // Set preemption settings
+ if set {
+ sq.quotaChangePreemptionDelay = conf.Preemption.Delay
+ }
+
+ // Reset preemption settings
+ if reset {
+ if sq.quotaChangePreemptionDelay != 0 {
+ sq.quotaChangePreemptionDelay = 0
+ }
+ }
Review Comment:
IMO it's simpler to move these one-liners to their respective `case`
branches, eg.
```
case resources.StrictlyGreaterThan(oldMaxResource, newMaxResource) &&
conf.Preemption.Delay != 0:
sq.quotaChangePreemptionDelay = conf.Preemption.Delay
```
and then we can get rid of `set` && `reset` vars.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
