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
no.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
no.2 is too much change and method signatures will be inconsistent (w/ and
w/o this extra data) and no.3 is too complicated. no.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]
