manirajv06 commented on code in PR #1039:
URL: https://github.com/apache/yunikorn-core/pull/1039#discussion_r2498002526
##########
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:
`nil` would simplify the checks and give us more grip on the caller side and
also correct way to express "nothing has been set" explicitly instead of empty
map initialised with default values. We have been doing the same for all Intra
queue preemption calculation as well.
Whereas, Returning `resources.Zero()` would require us to add more concrete
checks. For example, checking for "zero" value etc
##########
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:
Same as above
--
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]
