pbacsko commented on code in PR #1037:
URL: https://github.com/apache/yunikorn-core/pull/1037#discussion_r2461080396
##########
pkg/scheduler/objects/queue.go:
##########
@@ -89,9 +89,42 @@ type Queue struct {
template *template.Template
queueEvents *schedEvt.QueueEvents
+ // appID -> queuePath index
+ appIndex *appQueueMapping
+
+ locking.RWMutex
+}
+
+// appQueueMapping is a thread safe mapping from applicationID to queuePath
+type appQueueMapping struct {
+ byAppID map[string]string
locking.RWMutex
}
Review Comment:
This should be maintained globally, not in a per-queue basis. I think the
ideal place is `PartitionContext`. However, the data type has to stay in
`pkg/scheduler/objects` to avoid circular references.
Here is my take:
1. Make it public (`AppQueueMapping`)
2. Extract this type to a separate file eg.
`pkg/scheduler/objects/app_queue_mapping.go`, create simple unit tests for it
3. Create a single instance inside `PartitionContext` when the context is
created
4. When a `Queue` is created, inject the instance to the `Queue`. Extend
`NewConfiguredQueue()` and `NewRecoveryQueue()` and `NewDynamicQueue()` with an
extra argument.
5. Have a reference inside `Queue` to `appQueueMapping`
6. You can add/remove apps in `PartitionContext.AddApplication()` and
`PartitionContext.RemoveApplication()`
##########
pkg/scheduler/objects/queue.go:
##########
@@ -89,9 +89,42 @@ type Queue struct {
template *template.Template
queueEvents *schedEvt.QueueEvents
+ // appID -> queuePath index
+ appIndex *appQueueMapping
+
+ locking.RWMutex
+}
+
+// appQueueMapping is a thread safe mapping from applicationID to queuePath
+type appQueueMapping struct {
+ byAppID map[string]string
locking.RWMutex
}
Review Comment:
This should be maintained globally, not in a per-queue basis. I think the
ideal place is `PartitionContext`. However, the data type has to stay in
`pkg/scheduler/objects` to avoid circular references.
Here is my take:
1. Make it public (`AppQueueMapping`)
2. Extract this type to a separate file eg.
`pkg/scheduler/objects/app_queue_mapping.go`, create simple unit tests for it
3. Create a single instance inside `PartitionContext` when the context is
created
4. When a `Queue` is created, inject the instance to the `Queue`. Extend
`NewConfiguredQueue()` and `NewRecoveryQueue()` and `NewDynamicQueue()` with an
extra argument.
5. Have a reference inside `Queue` to `AppQueueMapping`
6. You can add/remove apps in `PartitionContext.AddApplication()` and
`PartitionContext.RemoveApplication()`
--
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]
