[GitHub] [incubator-yunikorn-core] manirajv06 commented on a change in pull request #356: [YUNIKORN-982] placeholder size checks

[GitBox <gi...@apache.org>]

manirajv06 commented on a change in pull request #356:
URL: https://github.com/apache/incubator-yunikorn-core/pull/356#discussion_r781247167



##########
File path: pkg/scheduler/partition.go
##########
@@ -704,6 +704,28 @@ func (pc *PartitionContext) removeNodeAllocations(node *objects.Node) ([]*object
 				if alloc.NodeID != release.NodeID {
 					// ignore the return as that is the same as alloc, the alloc is gone after this call
 					_ = app.ReplaceAllocation(allocID)
+					// we need to check the resources equality
+					delta := resources.Sub(release.AllocatedResource, alloc.AllocatedResource)
+					// Any negative value in the delta means that at least one of the requested resource in the
+					// placeholder is larger than the real allocation. The nodes are correct the queue needs adjusting.
+					// The reverse case is handled during allocation.
+					if delta.HasNegativeValue() {
+						// this looks incorrect but the delta is negative and the result will be a real decrease
+						err := app.GetQueue().IncAllocatedResource(delta, false)

Review comment:
       Would this new queue adjustment cause any issues in conjunction with below adjustment in place? app.GetQueue().DecAllocatedResource(alloc.AllocatedResource)
   
   Can we double check? May be unit test to removeNode with replacement on different node ensures this?
   
   Other than this, overall changes looks good.




-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org