wilfred-s commented on code in PR #1088:
URL: https://github.com/apache/yunikorn-core/pull/1088#discussion_r3346395332
##########
pkg/scheduler/objects/queue.go:
##########
@@ -253,6 +253,25 @@ func (sq *Queue) getProperties() map[string]string {
return props
}
+// GetProperties returns a copy of the properties for this queue.
+// Will never return nil; can return an empty map.
+func (sq *Queue) GetProperties() map[string]string {
+ return sq.getProperties()
+}
+
+// MergeParentProperties merges the parent queue properties with config
properties into this queue.
+// Config properties override parent properties. This should be called after
ApplyConf during
+// config reload to re-apply inherited properties from the parent.
+// Lock protected.
+func (sq *Queue) MergeParentProperties(config map[string]string) {
+ sq.Lock()
+ defer sq.Unlock()
Review Comment:
This is dangerous! In other locations we have the order of locks in the
opposite direction: parent first then child queue. This could cause a deadlock.
Solution: fix `getProperties()` by making it nil safe and use a read lock in
the function (currently a write lock). Move the `sq.parent.getProperties()`
call outside of the local queue lock
##########
pkg/scheduler/objects/queue_test.go:
##########
@@ -44,6 +44,167 @@ import (
)
// base test for creating a managed queue
+func TestMergeParentPropertiesNilParent(t *testing.T) {
+ // root queue has no parent, MergeParentProperties should be a no-op
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ root.properties = map[string]string{"key": "value"}
+
+ root.MergeParentProperties(map[string]string{"other": "other-value"})
+
+ // properties should remain unchanged since root has no parent to merge
from
+ props := root.GetProperties()
Review Comment:
Should not use the exported function (repeated all through the file)
--
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]
