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. 

##########
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:
       Actually, I may be mistaken. It looks like you're unconditionally 
removing and re-adding the node? If so, that also works.




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


Reply via email to