pbacsko commented on code in PR #1047:
URL: https://github.com/apache/yunikorn-core/pull/1047#discussion_r2522579718
##########
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:
My question was that why
`!qcp.allocatedResource.StrictlyGreaterThanOnlyExisting(qcp.guaranteedResource)`
is checked ONLY when guaranteed == max? Based on your explanation, it would be
logical to always check this if guaranteed is set, regardless of its relation
to max.
--
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]
