wilfred-s commented on code in PR #945:
URL: https://github.com/apache/yunikorn-core/pull/945#discussion_r1721610040
##########
pkg/scheduler/objects/preemption.go:
##########
@@ -326,7 +326,7 @@ func (p *Preemptor) calculateVictimsByNode(nodeAvailable
*resources.Resource, po
// removing task does not violate queue
constraints, adjust queue and node
nodeCurrentAvailable.AddTo(victim.GetAllocatedResource())
// check if ask now fits and we haven't
had this happen before
- if
nodeCurrentAvailable.FitInMaxUndef(p.ask.GetAllocatedResource()) && index < 0 {
+ if
nodeCurrentAvailable.FitIn(p.ask.GetAllocatedResource()) && index < 0 {
index = len(results)
}
Review Comment:
I am happy to do this without a test. We looked at adding a test but we need
to rip apart processing too much to be able to check the return value of the
method we are changing. The result of `calculateVictimsByNode` gets hidden too
much to be easily accessible. The setup required for testing the function
directly is not straight forward and fragile.
--
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]
