pbacsko commented on code in PR #723:
URL: https://github.com/apache/yunikorn-core/pull/723#discussion_r1400577268
##########
pkg/scheduler/objects/queue.go:
##########
@@ -369,30 +370,60 @@ func (sq *Queue) setResources(resource configs.Resources)
error {
guaranteedResource, err =
resources.NewResourceFromConf(resource.Guaranteed)
if err != nil {
log.Log(log.SchedQueue).Error("parsing failed on guaranteed
resources this should not happen",
+ zap.String("queue", sq.QueuePath),
zap.Error(err))
return err
}
- if resources.StrictlyGreaterThanZero(maxResource) {
+ switch {
+ case resources.StrictlyGreaterThanZero(maxResource):
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
if !resources.Equals(sq.maxResource, maxResource) &&
sq.queueEvents != nil {
sq.queueEvents.sendMaxResourceChangedEvent()
}
sq.maxResource = maxResource
sq.updateMaxResourceMetrics()
- } else {
- log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources")
+ case sq.maxResource != nil:
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
+ if sq.queueEvents != nil {
+ sq.queueEvents.sendMaxResourceChangedEvent()
+ }
+ sq.maxResource = nil
+ sq.updateMaxResourceMetrics()
+ default:
+ log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources", zap.String("queue", sq.QueuePath))
}
- if resources.StrictlyGreaterThanZero(guaranteedResource) {
+ switch {
+ case resources.StrictlyGreaterThanZero(guaranteedResource):
+ log.Log(log.SchedQueue).Debug("setting guaranteed resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current guaranteed res",
sq.guaranteedResource),
+ zap.Stringer("new guaranteed res", guaranteedResource))
if !resources.Equals(sq.guaranteedResource, guaranteedResource)
&& sq.queueEvents != nil {
sq.queueEvents.sendGuaranteedResourceChangedEvent()
}
sq.guaranteedResource = guaranteedResource
sq.updateGuaranteedResourceMetrics()
- } else {
- log.Log(log.SchedQueue).Debug("guaranteed resources setting
ignored: cannot set zero max resources")
+ case sq.guaranteedResource != nil:
+ log.Log(log.SchedQueue).Debug("setting guaranteed resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current guaranteed res",
sq.guaranteedResource),
+ zap.Stringer("new guaranteed res", guaranteedResource))
+ if sq.queueEvents != nil {
+ sq.queueEvents.sendGuaranteedResourceChangedEvent()
+ }
+ sq.guaranteedResource = nil
+ sq.updateGuaranteedResourceMetrics()
+ default:
+ log.Log(log.SchedQueue).Debug("guaranteed resources setting
ignored: cannot set zero max resources", zap.String("queue", sq.QueuePath))
Review Comment:
Same here, warning is OK. Also, "cannot set zero guaranteed resourced", not
"max".
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1266,8 +1297,31 @@ func (sq *Queue) SetMaxResource(max *resources.Resource)
{
log.Log(log.SchedQueue).Info("updating root queue max resources",
zap.Stringer("current max", sq.maxResource),
zap.Stringer("new max", max))
- sq.maxResource = max.Clone()
- sq.updateMaxResourceMetrics()
+
+ switch {
+ case resources.StrictlyGreaterThanZero(max):
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", max))
+ if !resources.Equals(sq.maxResource, max) && sq.queueEvents !=
nil {
+ sq.queueEvents.sendMaxResourceChangedEvent()
+ }
+ sq.maxResource = max.Clone()
+ sq.updateMaxResourceMetrics()
+ case sq.maxResource != nil:
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", max))
+ if sq.queueEvents != nil {
+ sq.queueEvents.sendMaxResourceChangedEvent()
+ }
+ sq.maxResource = nil
+ sq.updateMaxResourceMetrics()
+ default:
+ log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources", zap.String("queue", sq.QueuePath))
Review Comment:
Warning.
##########
pkg/scheduler/objects/queue.go:
##########
@@ -369,30 +370,60 @@ func (sq *Queue) setResources(resource configs.Resources)
error {
guaranteedResource, err =
resources.NewResourceFromConf(resource.Guaranteed)
if err != nil {
log.Log(log.SchedQueue).Error("parsing failed on guaranteed
resources this should not happen",
+ zap.String("queue", sq.QueuePath),
zap.Error(err))
return err
}
- if resources.StrictlyGreaterThanZero(maxResource) {
+ switch {
+ case resources.StrictlyGreaterThanZero(maxResource):
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
if !resources.Equals(sq.maxResource, maxResource) &&
sq.queueEvents != nil {
sq.queueEvents.sendMaxResourceChangedEvent()
}
sq.maxResource = maxResource
sq.updateMaxResourceMetrics()
- } else {
- log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources")
+ case sq.maxResource != nil:
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
+ if sq.queueEvents != nil {
+ sq.queueEvents.sendMaxResourceChangedEvent()
+ }
+ sq.maxResource = nil
+ sq.updateMaxResourceMetrics()
+ default:
+ log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources", zap.String("queue", sq.QueuePath))
Review Comment:
I think this can easily be a warning
##########
pkg/scheduler/objects/queue.go:
##########
@@ -369,30 +370,60 @@ func (sq *Queue) setResources(resource configs.Resources)
error {
guaranteedResource, err =
resources.NewResourceFromConf(resource.Guaranteed)
if err != nil {
log.Log(log.SchedQueue).Error("parsing failed on guaranteed
resources this should not happen",
+ zap.String("queue", sq.QueuePath),
zap.Error(err))
return err
}
- if resources.StrictlyGreaterThanZero(maxResource) {
+ switch {
+ case resources.StrictlyGreaterThanZero(maxResource):
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
Review Comment:
"current", "new"
##########
pkg/scheduler/objects/queue.go:
##########
@@ -369,30 +370,60 @@ func (sq *Queue) setResources(resource configs.Resources)
error {
guaranteedResource, err =
resources.NewResourceFromConf(resource.Guaranteed)
if err != nil {
log.Log(log.SchedQueue).Error("parsing failed on guaranteed
resources this should not happen",
+ zap.String("queue", sq.QueuePath),
zap.Error(err))
return err
}
- if resources.StrictlyGreaterThanZero(maxResource) {
+ switch {
+ case resources.StrictlyGreaterThanZero(maxResource):
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
if !resources.Equals(sq.maxResource, maxResource) &&
sq.queueEvents != nil {
sq.queueEvents.sendMaxResourceChangedEvent()
}
sq.maxResource = maxResource
sq.updateMaxResourceMetrics()
- } else {
- log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources")
+ case sq.maxResource != nil:
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
+ zap.Stringer("new max res", maxResource))
+ if sq.queueEvents != nil {
+ sq.queueEvents.sendMaxResourceChangedEvent()
+ }
+ sq.maxResource = nil
+ sq.updateMaxResourceMetrics()
+ default:
+ log.Log(log.SchedQueue).Debug("max resources setting ignored:
cannot set zero max resources", zap.String("queue", sq.QueuePath))
}
- if resources.StrictlyGreaterThanZero(guaranteedResource) {
+ switch {
+ case resources.StrictlyGreaterThanZero(guaranteedResource):
+ log.Log(log.SchedQueue).Debug("setting guaranteed resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current guaranteed res",
sq.guaranteedResource),
Review Comment:
"current", "new"
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1266,8 +1297,31 @@ func (sq *Queue) SetMaxResource(max *resources.Resource)
{
log.Log(log.SchedQueue).Info("updating root queue max resources",
zap.Stringer("current max", sq.maxResource),
zap.Stringer("new max", max))
- sq.maxResource = max.Clone()
- sq.updateMaxResourceMetrics()
+
+ switch {
+ case resources.StrictlyGreaterThanZero(max):
+ log.Log(log.SchedQueue).Debug("setting max resources",
+ zap.String("queue", sq.QueuePath),
+ zap.Stringer("current max res", sq.maxResource),
Review Comment:
"current", "new"
--
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]
