pbacsko commented on code in PR #935:
URL: https://github.com/apache/yunikorn-core/pull/935#discussion_r1709966355
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1220,6 +1220,63 @@ func (sq *Queue) GetMaxResource() *resources.Resource {
return sq.internalGetMax(limit)
}
+// Starting with the root, descend down to the target queue allowing children
to override Resource values .
+// If the root includes an explicit 0 value for a Resource, do not include it
in the accumulator and treat it as missing.
+// If no children provide a maximum capacity override, the resulting value
will be the value found on the Root.
+// It is useful for fair-scheduling to allow a ratio to be produced
representing the rough utilization % of a given queue.
+func (sq *Queue) GetFairMaxResource() *resources.Resource {
+ var limit *resources.Resource
+ if sq.parent == nil {
+ cleanedRootResources := resources.NewResource()
Review Comment:
Nit: no need to use long variable names in tiny scopes. Just call it "ret"
or "out" or even "cleaned".
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1220,6 +1220,63 @@ func (sq *Queue) GetMaxResource() *resources.Resource {
return sq.internalGetMax(limit)
}
+// Starting with the root, descend down to the target queue allowing children
to override Resource values .
+// If the root includes an explicit 0 value for a Resource, do not include it
in the accumulator and treat it as missing.
+// If no children provide a maximum capacity override, the resulting value
will be the value found on the Root.
+// It is useful for fair-scheduling to allow a ratio to be produced
representing the rough utilization % of a given queue.
+func (sq *Queue) GetFairMaxResource() *resources.Resource {
+ var limit *resources.Resource
+ if sq.parent == nil {
+ cleanedRootResources := resources.NewResource()
+ if sq.maxResource == nil {
+ return cleanedRootResources
+ }
+
+ for k, v := range sq.maxResource.Resources {
+ if v != 0 {
+ cleanedRootResources.Resources[k] = v
+ }
+ }
+
+ return cleanedRootResources
+ }
+
+ limit = sq.parent.GetFairMaxResource()
+ return sq.internalGetFairMaxResource(limit)
+}
+
+func (sq *Queue) internalGetFairMaxResource(parent *resources.Resource)
*resources.Resource {
+ sq.RLock()
+ defer sq.RUnlock()
+
+ us := sq.maxResource
+
+ if parent == nil && us == nil {
+ return nil
+ }
+ if parent == nil {
+ return us.Clone()
+ }
+ if us == nil {
+ return parent.Clone()
+ }
+
+ out := resources.NewResource()
+ for k, v := range parent.Resources {
+ out.Resources[k] = v
+ }
Review Comment:
Isn't this just simple cloning here?
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1220,6 +1220,63 @@ func (sq *Queue) GetMaxResource() *resources.Resource {
return sq.internalGetMax(limit)
}
+// Starting with the root, descend down to the target queue allowing children
to override Resource values .
+// If the root includes an explicit 0 value for a Resource, do not include it
in the accumulator and treat it as missing.
+// If no children provide a maximum capacity override, the resulting value
will be the value found on the Root.
+// It is useful for fair-scheduling to allow a ratio to be produced
representing the rough utilization % of a given queue.
+func (sq *Queue) GetFairMaxResource() *resources.Resource {
+ var limit *resources.Resource
+ if sq.parent == nil {
+ cleanedRootResources := resources.NewResource()
+ if sq.maxResource == nil {
+ return cleanedRootResources
+ }
+
+ for k, v := range sq.maxResource.Resources {
+ if v != 0 {
+ cleanedRootResources.Resources[k] = v
+ }
+ }
+
+ return cleanedRootResources
+ }
+
+ limit = sq.parent.GetFairMaxResource()
+ return sq.internalGetFairMaxResource(limit)
+}
+
+func (sq *Queue) internalGetFairMaxResource(parent *resources.Resource)
*resources.Resource {
+ sq.RLock()
+ defer sq.RUnlock()
+
+ us := sq.maxResource
+
+ if parent == nil && us == nil {
+ return nil
+ }
+ if parent == nil {
+ return us.Clone()
+ }
+ if us == nil {
+ return parent.Clone()
+ }
+
+ out := resources.NewResource()
+ for k, v := range parent.Resources {
+ out.Resources[k] = v
+ }
+
+ //child wins every resources collision
+ for k, v := range us.Resources {
+ log.Log(log.SchedQueue).Info("child overwriting parent",
+ zap.Any("k", k),
+ zap.Any("v", v))
+
+ out.Resources[k] = v
+ }
Review Comment:
I think this entire chain can written in a more Go-like fashion:
```
var out *resources.Resources
switch {
case parent == nil && us == nil:
// nop
case parent == nil:
out = us.Clone()
case us == nil:
out = parent.Clone()
default:
out = parent.Clone() // see my question above
for k, v := range us.Resources {
out.Resources[k] = v // overwrite parent values with child values
}
}
return out
```
--
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]
