Skip to content

Commit 6ea79d9

Browse files
committed
simplified logic around merging properties
1 parent 146c011 commit 6ea79d9

3 files changed

Lines changed: 13 additions & 15 deletions

File tree

pkg/scheduler/objects/queue.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -253,20 +253,18 @@ func (sq *Queue) getProperties() map[string]string {
253253
return props
254254
}
255255

256-
// GetProperties returns a copy of the properties for this queue.
257-
// Will never return nil; can return an empty map.
258-
func (sq *Queue) GetProperties() map[string]string {
259-
return sq.getProperties()
260-
}
261-
262-
// MergeProperties merges the parent queue properties with config properties into this queue.
263-
// Config properties override parent properties. This should be called after ApplyConf during
264-
// config reload to re-apply inherited properties from the parent.
265-
// Lock protected.
266-
func (sq *Queue) MergeProperties(parent, config map[string]string) {
256+
// ApplyInheritedProperties merges the parent queue's properties with the given config properties
257+
// into this queue. Config properties override parent properties. This should be called after
258+
// ApplyConf during config reload to re-apply inherited properties from the parent.
259+
// Parent properties are fetched before acquiring sq's lock to avoid lock-ordering issues.
260+
func (sq *Queue) ApplyInheritedProperties(parent *Queue, config map[string]string) {
261+
var parentProps map[string]string
262+
if parent != nil {
263+
parentProps = parent.getProperties()
264+
}
267265
sq.Lock()
268266
defer sq.Unlock()
269-
sq.mergeProperties(parent, config)
267+
sq.mergeProperties(parentProps, config)
270268
}
271269

272270
// mergeProperties merges the properties from the parent queue and the config in the set from new queue

pkg/scheduler/partition.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ func (pc *PartitionContext) updateQueues(config []configs.QueueConfig, parent *o
246246
// ApplyConf sets sq.properties to only the queue's own config properties, which
247247
// drops any previously inherited values and prevents parent property changes from
248248
// propagating to existing child queues on config reload.
249-
queue.MergeProperties(parent.GetProperties(), queueConfig.Properties)
249+
queue.ApplyInheritedProperties(parent, queueConfig.Properties)
250250
}
251251
}
252252
if err != nil {

pkg/scheduler/partition_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,7 +1460,7 @@ func TestUpdateQueuesInheritedQuotaPreemptionDelay(t *testing.T) {
14601460

14611461
leaf := partition.GetQueue("root.parent.leaf")
14621462
assert.Assert(t, leaf != nil, "leaf queue should exist")
1463-
assert.Equal(t, leaf.GetProperties()[configs.QuotaPreemptionDelay], "20s",
1463+
assert.Equal(t, leaf.GetPartitionQueueDAOInfo(false).Properties[configs.QuotaPreemptionDelay], "20s",
14641464
"leaf should inherit quota.preemption.delay from parent on initial load")
14651465

14661466
// config reload: parent changes delay to 30s; leaf should pick up the new value
@@ -1484,7 +1484,7 @@ func TestUpdateQueuesInheritedQuotaPreemptionDelay(t *testing.T) {
14841484

14851485
leaf = partition.GetQueue("root.parent.leaf")
14861486
assert.Assert(t, leaf != nil, "leaf queue should still exist after reload")
1487-
assert.Equal(t, leaf.GetProperties()[configs.QuotaPreemptionDelay], "30s",
1487+
assert.Equal(t, leaf.GetPartitionQueueDAOInfo(false).Properties[configs.QuotaPreemptionDelay], "30s",
14881488
"leaf should inherit updated quota.preemption.delay from parent on config reload")
14891489
assert.Equal(t, leaf.GetQuotaPreemptionDelay(), 30*time.Second,
14901490
"leaf's runtime quotaPreemptionDelay should reflect the updated inherited value")

0 commit comments

Comments
 (0)