You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by wi...@apache.org on 2026/06/05 06:59:30 UTC
(yunikorn-core) branch master updated: [YUNIKORN-3285] Child queue property inheritance in update (#1088)
This is an automated email from the ASF dual-hosted git repository.
wilfred-s pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git
The following commit(s) were added to refs/heads/master by this push:
new fae6e256 [YUNIKORN-3285] Child queue property inheritance in update (#1088)
fae6e256 is described below
commit fae6e256cd5c29c7249da1c6999037aa13f57a91
Author: Aditya Maheshwari <ad...@gmail.com>
AuthorDate: Fri Jun 5 16:55:00 2026 +1000
[YUNIKORN-3285] Child queue property inheritance in update (#1088)
During the update of a child queue the inherited properties from the
parent are not correctly updated and lost. The create and update process
should follow the same steps:
* inherit filtered parent properties
* override with child properties
All properties set should be based on the configuration passed in, even
for the root queue.
Closes: #1088
Signed-off-by: Wilfred Spiegelenburg <wi...@apache.org>
---
pkg/scheduler/objects/queue.go | 25 +++++-
pkg/scheduler/objects/queue_test.go | 161 ++++++++++++++++++++++++++++++++++++
pkg/scheduler/partition.go | 7 ++
pkg/scheduler/partition_test.go | 60 ++++++++++++++
4 files changed, 251 insertions(+), 2 deletions(-)
diff --git a/pkg/scheduler/objects/queue.go b/pkg/scheduler/objects/queue.go
index 9fac1184..2fa9cca1 100644
--- a/pkg/scheduler/objects/queue.go
+++ b/pkg/scheduler/objects/queue.go
@@ -244,15 +244,36 @@ func (sq *Queue) applyTemplate(childTemplate *template.Template) {
// getProperties returns a copy of the properties for this queue
// Will never return a nil, can return an empty map.
func (sq *Queue) getProperties() map[string]string {
- sq.Lock()
- defer sq.Unlock()
props := make(map[string]string)
+ if sq == nil {
+ return props
+ }
+ sq.RLock()
+ defer sq.RUnlock()
for key, value := range sq.properties {
props[key] = value
}
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) {
+ // get parent properties outside of the lock to avoid potential deadlocks with parent queue lock.
+ parentProps := sq.parent.getProperties()
+ sq.Lock()
+ defer sq.Unlock()
+ sq.mergeProperties(parentProps, config)
+}
+
// mergeProperties merges the properties from the parent queue and the config in the set from new queue
// lock free call
func (sq *Queue) mergeProperties(parent, config map[string]string) {
diff --git a/pkg/scheduler/objects/queue_test.go b/pkg/scheduler/objects/queue_test.go
index 4718ae29..ef453128 100644
--- a/pkg/scheduler/objects/queue_test.go
+++ b/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
+ 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"})
+
+ props := root.getProperties()
+ _, exists := props["key"]
+ assert.Assert(t, !exists, "existing properties should not be inherited when parent is nil")
+ value, exists := props["other"]
+ assert.Assert(t, exists, "config props should be applied even when parent is nil")
+ assert.Equal(t, "other-value", value, "config props should be applied even when parent is nil")
+}
+
+func TestMergeParentPropertiesParentPropsInherited(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{"inherited-key": "inherited-value"})
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ child.MergeParentProperties(nil)
+
+ props := child.getProperties()
+ assert.Equal(t, "inherited-value", props["inherited-key"], "child should inherit parent properties")
+}
+
+func TestMergeParentPropertiesConfigOverridesParent(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{"key": "parent-value"})
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ child.MergeParentProperties(map[string]string{"key": "config-value", "extra": "extra-value"})
+
+ props := child.getProperties()
+ assert.Equal(t, "config-value", props["key"], "config property should override parent property")
+ assert.Equal(t, "extra-value", props["extra"], "config-only property should be present")
+}
+
+func TestMergeParentPropertiesClearsOldProperties(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueue(root, "parent", true, nil)
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ // manually set a stale property that should be wiped on next merge
+ child.Lock()
+ child.properties["stale-key"] = "stale-value"
+ child.Unlock()
+
+ child.MergeParentProperties(map[string]string{"new-key": "new-value"})
+
+ props := child.getProperties()
+ _, exists := props["stale-key"]
+ assert.Assert(t, !exists, "stale properties should be cleared on MergeParentProperties")
+ assert.Equal(t, "new-value", props["new-key"], "new config property should be present")
+}
+
+func TestMergeParentPropertiesFilterPriorityPolicy(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{
+ configs.PriorityPolicy: policies.FencePriorityPolicy.String(),
+ })
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ child.MergeParentProperties(nil)
+
+ props := child.getProperties()
+ // priority.policy should not be inherited from parent; filterParentProperty resets it to default
+ assert.Equal(t, policies.DefaultPriorityPolicy.String(), props[configs.PriorityPolicy],
+ "priority.policy should be reset to default when inherited from parent")
+}
+
+func TestMergeParentPropertiesFilterPriorityOffset(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{
+ configs.PriorityOffset: "10",
+ })
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ child.MergeParentProperties(nil)
+
+ props := child.getProperties()
+ // priority.offset should not be inherited; filterParentProperty resets it to "0"
+ assert.Equal(t, "0", props[configs.PriorityOffset],
+ "priority.offset should be reset to 0 when inherited from parent")
+}
+
+func TestMergeParentPropertiesFilterPreemptionPolicyDisabledPropagates(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{
+ configs.PreemptionPolicy: policies.DisabledPreemptionPolicy.String(),
+ })
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ child.MergeParentProperties(nil)
+
+ props := child.getProperties()
+ // disabled preemption.policy is the only value that propagates from parent
+ assert.Equal(t, policies.DisabledPreemptionPolicy.String(), props[configs.PreemptionPolicy],
+ "disabled preemption.policy should propagate from parent")
+}
+
+func TestMergeParentPropertiesFilterPreemptionPolicyNonDisabledReset(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{
+ configs.PreemptionPolicy: policies.FencePreemptionPolicy.String(),
+ })
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ child.MergeParentProperties(nil)
+
+ props := child.getProperties()
+ // non-disabled preemption.policy should not propagate from parent; reset to default
+ assert.Equal(t, policies.DefaultPreemptionPolicy.String(), props[configs.PreemptionPolicy],
+ "non-disabled preemption.policy should be reset to default when inherited from parent")
+}
+
+func TestMergeParentPropertiesConfigCanOverrideFilteredProps(t *testing.T) {
+ root, err := createRootQueue(nil)
+ assert.NilError(t, err, "root queue create failed")
+ parent, err := createManagedQueueWithProps(root, "parent", true, nil, map[string]string{
+ configs.PriorityPolicy: policies.FencePriorityPolicy.String(),
+ configs.PriorityOffset: "10",
+ })
+ assert.NilError(t, err, "parent queue create failed")
+ child, err := createManagedQueue(parent, "leaf", false, nil)
+ assert.NilError(t, err, "child queue create failed")
+
+ // config explicitly sets these, so they should not be overridden by the parent filter
+ child.MergeParentProperties(map[string]string{
+ configs.PriorityPolicy: policies.FencePriorityPolicy.String(),
+ configs.PriorityOffset: "5",
+ })
+
+ props := child.getProperties()
+ assert.Equal(t, policies.FencePriorityPolicy.String(), props[configs.PriorityPolicy],
+ "config priority.policy should override filtered parent value")
+ assert.Equal(t, "5", props[configs.PriorityOffset],
+ "config priority.offset should override filtered parent value")
+}
+
func TestQueueBasics(t *testing.T) {
// create the root
root, err := createRootQueue(nil)
diff --git a/pkg/scheduler/partition.go b/pkg/scheduler/partition.go
index 94095e28..7daa94eb 100644
--- a/pkg/scheduler/partition.go
+++ b/pkg/scheduler/partition.go
@@ -241,6 +241,13 @@ func (pc *PartitionContext) updateQueues(config []configs.QueueConfig, parent *o
queue, err = objects.NewConfiguredQueue(queueConfig, parent, false, pc.appQueueMapping)
} else {
oldMax, err = queue.ApplyConf(queueConfig)
+ if err == nil {
+ // Re-apply inherited properties from parent, mirroring the NewConfiguredQueue path.
+ // ApplyConf sets sq.properties to only the queue's own config properties, which
+ // drops any previously inherited values and prevents parent property changes from
+ // propagating to existing child queues on config reload.
+ queue.MergeParentProperties(queueConfig.Properties)
+ }
}
if err != nil {
return err
diff --git a/pkg/scheduler/partition_test.go b/pkg/scheduler/partition_test.go
index 9d79bd84..49949786 100644
--- a/pkg/scheduler/partition_test.go
+++ b/pkg/scheduler/partition_test.go
@@ -1428,6 +1428,66 @@ func TestUpdateQueues(t *testing.T) {
assertUpdateQueues(t, "both", map[string]string{})
}
+// TestUpdateQueuesInheritedProperties verifies that a child queue inherits
+// properties from its parent on config reload (updateQueues path).
+func TestUpdateQueuesInheritedProperties(t *testing.T) {
+ const customProp = "custom.test.property"
+
+ initialConf := []configs.QueueConfig{
+ {
+ Name: "parent",
+ Parent: true,
+ Properties: map[string]string{
+ customProp: "value1",
+ },
+ Queues: []configs.QueueConfig{
+ {
+ Name: "leaf",
+ Parent: false,
+ },
+ },
+ },
+ }
+
+ partition, err := newBasePartition()
+ assert.NilError(t, err, "partition create failed")
+ root := partition.GetQueue("root")
+ assert.Assert(t, root != nil, "root queue not found")
+
+ // initial load: leaf should inherit the property from parent
+ err = partition.updateQueues(initialConf, root)
+ assert.NilError(t, err, "initial updateQueues failed")
+
+ leaf := partition.GetQueue("root.parent.leaf")
+ assert.Assert(t, leaf != nil, "leaf queue should exist")
+ assert.Equal(t, leaf.GetProperties()[customProp], "value1",
+ "leaf should inherit property from parent on initial load")
+
+ // config reload: parent changes property value; leaf should pick up the new value
+ updatedConf := []configs.QueueConfig{
+ {
+ Name: "parent",
+ Parent: true,
+ Properties: map[string]string{
+ customProp: "value2",
+ },
+ Queues: []configs.QueueConfig{
+ {
+ Name: "leaf",
+ Parent: false,
+ },
+ },
+ },
+ }
+ err = partition.updateQueues(updatedConf, root)
+ assert.NilError(t, err, "config-reload updateQueues failed")
+
+ leaf = partition.GetQueue("root.parent.leaf")
+ assert.Assert(t, leaf != nil, "leaf queue should still exist after reload")
+ assert.Equal(t, leaf.GetProperties()[customProp], "value2",
+ "leaf should inherit updated property from parent on config reload")
+}
+
func TestReAddQueues(t *testing.T) {
conf := []configs.QueueConfig{
{
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org