manirajv06 commented on code in PR #1047:
URL: https://github.com/apache/yunikorn-core/pull/1047#discussion_r2521872568
##########
pkg/scheduler/objects/quota_change_preemptor.go:
##########
@@ -149,24 +164,39 @@ func (qcp *QuotaChangePreemptionContext) preemptVictims()
{
zap.Int("total victims", len(qcp.allocations)))
apps := make(map[*Application][]*Allocation)
victimsTotalResource := resources.NewResource()
- selectedVictimsTotalResource := resources.NewResource()
+ isGuaranteedAndMaxEquals := qcp.maxResource != nil &&
qcp.guaranteedResource != nil && resources.Equals(qcp.maxResource,
qcp.guaranteedResource)
+ log.Log(log.ShedQuotaChangePreemption).Info("Found victims for quota
change preemption",
+ zap.String("queue", qcp.queue.GetQueuePath()),
+ zap.Int("total victims", len(qcp.allocations)),
+ zap.String("max resources", qcp.maxResource.String()),
+ zap.String("guaranteed resources",
qcp.guaranteedResource.String()),
+ zap.String("allocated resources",
qcp.allocatedResource.String()),
+ zap.String("preemptable resources",
qcp.preemptableResource.String()),
+ zap.Bool("isGuaranteedSet", isGuaranteedAndMaxEquals),
+ )
for _, victim := range qcp.allocations {
- // stop collecting the victims once ask resource requirement met
+ if
!qcp.preemptableResource.FitInMaxUndef(victim.GetAllocatedResource()) {
+ continue
+ }
+ application := qcp.queue.GetApplication(victim.applicationID)
+
+ // Keep collecting the victims until preemptable resource
reaches and subtract the usage
if
qcp.preemptableResource.StrictlyGreaterThanOnlyExisting(victimsTotalResource) {
- application :=
qcp.queue.GetApplication(victim.applicationID)
- if _, ok := apps[application]; !ok {
- apps[application] = []*Allocation{}
- }
apps[application] = append(apps[application], victim)
-
selectedVictimsTotalResource.AddTo(victim.GetAllocatedResource())
+
qcp.allocatedResource.SubFrom(victim.GetAllocatedResource())
}
- victimsTotalResource.AddTo(victim.GetAllocatedResource())
- }
- if
qcp.preemptableResource.StrictlyGreaterThanOnlyExisting(victimsTotalResource) ||
-
selectedVictimsTotalResource.StrictlyGreaterThanOnlyExisting(qcp.preemptableResource)
{
- // either there is a shortfall or exceeding little above than
required, so try "best effort" approach later
- return
+ // Has usage gone below the guaranteed resources?
+ // If yes, revert the recently added victim steps completely
and try next victim.
+ if isGuaranteedAndMaxEquals &&
!qcp.allocatedResource.StrictlyGreaterThanOnlyExisting(qcp.guaranteedResource) {
Review Comment:
BTW, even in regular preemption, we don't preempt victims if it is not
suitable. Suitability depends on various factors - 1. Sum of the resources used
by victims is sufficient enough or not 2. Are all victims running on the same
node or not? If not, whether node has extra space or not etc.
--
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]
