[GitHub] [incubator-yunikorn-core] craigcondit commented on a change in pull request #307: [YUNIKORN-807] Improve performance of node sorting.

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

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



##########
File path: pkg/scheduler/objects/node.go
##########
@@ -128,6 +130,7 @@ func (sn *Node) GetCapacity() *resources.Resource {
 }
 
 func (sn *Node) SetCapacity(newCapacity *resources.Resource) *resources.Resource {
+	defer sn.notifyListeners()

Review comment:
       @yangwwei, you are correct that notification is not strictly necessary in the short-circuit return. However, it is vital to avoid deadlocks that notifyListeners() is only called when the lock is not held, which is why we defer the notifyListeners() call to the end. defer statements are executed last-in, first-out, so this ensures that we have releases our lock before calling it. Conversely, we need the lock to be able to check the condition in line 137, so we have a chicken-and-egg problem with clearly taking / releasing locks when needed. The current approach may not be 100% optimal, but it is easy to reason about and causes no locking or deadlock issues.




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