pbacsko commented on code in PR #1039:
URL: https://github.com/apache/yunikorn-core/pull/1039#discussion_r2493875461


##########
pkg/scheduler/objects/quota_change_preemptor.go:
##########
@@ -48,3 +54,26 @@ func (qcp *QuotaChangePreemptionContext) tryPreemption() {
        // quota change preemption has really evicted victims, so mark the flag
        qcp.queue.MarkTriggerredQuotaChangePreemption()
 }
+
+// GetPreemptableResources Get the preemptable resources for the queue
+// Subtracting the usage from the max resource gives the preemptable resources.
+// It could contain both positive and negative values. Only negative values 
are preemptable.
+func (qcp *QuotaChangePreemptionContext) GetPreemptableResources() 
*resources.Resource {
+       maxRes := qcp.queue.CloneMaxResource()
+       used := resources.SubOnlyExisting(qcp.queue.GetAllocatedResource(), 
qcp.queue.GetPreemptingResource())
+       if maxRes.IsEmpty() || used.IsEmpty() {
+               return nil

Review Comment:
   Nit: wouldn't it be safer to always return something instead of "nil"? To 
me, a `resources.Zero()` seems to be more appropriate.



##########
pkg/scheduler/objects/quota_change_preemptor.go:
##########
@@ -48,3 +54,26 @@ func (qcp *QuotaChangePreemptionContext) tryPreemption() {
        // quota change preemption has really evicted victims, so mark the flag
        qcp.queue.MarkTriggerredQuotaChangePreemption()
 }
+
+// GetPreemptableResources Get the preemptable resources for the queue
+// Subtracting the usage from the max resource gives the preemptable resources.
+// It could contain both positive and negative values. Only negative values 
are preemptable.
+func (qcp *QuotaChangePreemptionContext) GetPreemptableResources() 
*resources.Resource {
+       maxRes := qcp.queue.CloneMaxResource()
+       used := resources.SubOnlyExisting(qcp.queue.GetAllocatedResource(), 
qcp.queue.GetPreemptingResource())
+       if maxRes.IsEmpty() || used.IsEmpty() {
+               return nil
+       }
+       actual := resources.SubOnlyExisting(maxRes, used)
+       preemptableResource := resources.NewResource()
+       // Keep only the resource type which needs to be preempted
+       for k, v := range actual.Resources {
+               if v < 0 {
+                       preemptableResource.Resources[k] = 
resources.Quantity(math.Abs(float64(v)))
+               }
+       }
+       if preemptableResource.IsEmpty() {
+               return nil
+       }

Review Comment:
   Ditto



-- 
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]

Reply via email to