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.
##########
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:
Makes sense.
--
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]
