pbacsko commented on code in PR #548:
URL: https://github.com/apache/yunikorn-core/pull/548#discussion_r1219415291
##########
pkg/scheduler/objects/node_iterator.go:
##########
@@ -77,6 +81,42 @@ func NewDefaultNodeIterator(schedulerNodes []*Node)
NodeIterator {
return it
}
+type treeIterator struct {
Review Comment:
An alternative approach with GODS Btree/Redblacktree (don't want to code
this): those impls have an {{Iterator}} interface (see
https://github.com/emirpasic/gods/blob/master/trees/redblacktree/iterator.go)
but don't have {{Clone()}}, so we need to lock a mutex while iterating. The
only drawback is that we can receive node updates from different code paths,
ie. you might or might not need to obtain a mutex. If you already called
`Lock()`, you must not call it again, otherwise it'll be a deadlock. This can
be solved in three ways at least (maybe there are some more which involves
trickery, but #3 is already somewhat involved):
1. run `baseNodeCollection.NodeUpdated()` calls on a separate goroutine
where it receives `Node` objects on a channel
2. propagate a `bool` or a boolean embedded in a `Context` to signal whether
we already have a lock or not
3. use CAS + Wait-Notify combo to mark if an iteration is in progress and
grab locks accordingly
#2 is too much change and method signatures will be inconsistent (w/ and w/o
this extra data) and #3 is too complicated. #1 is nice, we detach the update
logic from the scheduling cycle (it does not need to be synchronous).
The upside of using GODS collections is that the nature of iterator will
remain, we still use the `Next()` and `HastNext()` methods, it needs less
change in other files.
--
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]
