wilfred-s commented on code in PR #1086:
URL: https://github.com/apache/yunikorn-core/pull/1086#discussion_r3349802047
##########
pkg/scheduler/objects/queue.go:
##########
@@ -662,6 +662,18 @@ func (sq *Queue) UpdateQueueProperties(oldMaxResource
*resources.Resource) {
sq.sortType = policies.FifoSortPolicy
return
}
+ // Reset fields derived from properties so removed properties revert to
defaults.
Review Comment:
Can we move setting all these defaults into a single function:
`sq.resetProperties()` ?
##########
pkg/scheduler/objects/queue.go:
##########
@@ -727,20 +739,26 @@ func (sq *Queue) UpdateQueueProperties(oldMaxResource
*resources.Resource) {
zap.Error(err))
}
case configs.QuotaPreemptionDelay:
- oldDelay := sq.quotaPreemptionDelay
+ quotaPreemptionDelaySet = true
sq.quotaPreemptionDelay, err = convertDelay(value,
configs.DefaultQuotaPreemptionDelay)
if err != nil {
log.Log(log.SchedQueue).Debug("quota preemption
delay configuration error",
zap.Error(err))
}
- sq.setPreemptionTime(oldMaxResource, oldDelay)
default:
// skip unknown properties just log them
log.Log(log.SchedQueue).Debug("queue property skipped",
zap.String("key", key),
zap.String("value", value))
}
}
+ if quotaPreemptionDelaySet || oldDelay != sq.quotaPreemptionDelay {
+ if sq.quotaPreemptionDelay == 0 {
+ sq.quotaPreemptionStartTime = time.Time{}
+ return
+ }
+ sq.setPreemptionTime(oldMaxResource, oldDelay)
+ }
Review Comment:
This can be hidden inside the `sq.setPreemptionTime(oldMaxResource,
oldDelay)` call and always call that method with a small change inside that
method:
* if preemption is running the start time will be cleaned up automatically
* if preemption is not running and delay is not set now clear out the start
time
* all other cases let it flow as it was
Removes the need to track `quotaPreemptionDelaySet`
--
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]
