[GitHub] [incubator-yunikorn-core] craigcondit commented on a change in pull request #318: [YUNIKORN-831] node sorting should check other resources of nodes ins…

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

craigcondit commented on a change in pull request #318:
URL: https://github.com/apache/incubator-yunikorn-core/pull/318#discussion_r699350349



##########
File path: pkg/scheduler/objects/node_collection.go
##########
@@ -211,13 +213,7 @@ func (nc *baseNodeCollection) NodeUpdated(node *Node) {
 	nc.Lock()
 	defer nc.Unlock()
 
-	nref := nc.nodes[node.NodeID]
-	if nref == nil {
-		return
-	}
-
-	updatedScore := nc.scoreNode(node)
-	if nref.nodeScore != updatedScore {
+	if nref, ok := nc.nodes[node.NodeID]; ok {

Review comment:
       I don't think this is correct. When NodeUpdated() is called, it might be due to usage values changing, and therefore we need to calculate a new score and update nref.nodeScore. If the score has changed, the node needs to be removed from the BTree (looked up using previous version of nref) and re-added using the updated nref. 




-- 
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