Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions pkg/scheduler/objects/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,18 @@ func (sq *Queue) UpdateQueueProperties(oldMaxResource *resources.Resource) {
sq.sortType = policies.FifoSortPolicy
return
}
// Reset fields derived from properties so removed properties revert to defaults.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move setting all these defaults into a single function: sq.resetProperties() ?

sq.sortType = policies.FifoSortPolicy
sq.prioritySortEnabled = true
sq.priorityOffset = 0
sq.priorityPolicy = policies.DefaultPriorityPolicy
sq.preemptionPolicy = policies.DefaultPreemptionPolicy
sq.preemptionDelay = configs.DefaultPreemptionDelay
sq.unschedAskBackoff = 0
sq.askBackoffDelay = configs.DefaultAskBackOffDelay
oldDelay := sq.quotaPreemptionDelay
sq.quotaPreemptionDelay = configs.DefaultQuotaPreemptionDelay
quotaPreemptionDelaySet := false
if !sq.isLeaf {
// set the sorting type for parent queues
sq.sortType = policies.FairSortPolicy
Expand Down Expand Up @@ -727,20 +739,26 @@ func (sq *Queue) UpdateQueueProperties(oldMaxResource *resources.Resource) {
zap.Error(err))
}
case configs.QuotaPreemptionDelay:
oldDelay := sq.quotaPreemptionDelay
quotaPreemptionDelaySet = true
sq.quotaPreemptionDelay, err = convertDelay(value, configs.DefaultQuotaPreemptionDelay)
if err != nil {
log.Log(log.SchedQueue).Debug("quota preemption delay configuration error",
zap.Error(err))
}
sq.setPreemptionTime(oldMaxResource, oldDelay)
default:
// skip unknown properties just log them
log.Log(log.SchedQueue).Debug("queue property skipped",
zap.String("key", key),
zap.String("value", value))
}
}
if quotaPreemptionDelaySet || oldDelay != sq.quotaPreemptionDelay {
if sq.quotaPreemptionDelay == 0 {
sq.quotaPreemptionStartTime = time.Time{}
return
}
sq.setPreemptionTime(oldMaxResource, oldDelay)
}
Comment on lines +755 to +761
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be hidden inside the sq.setPreemptionTime(oldMaxResource, oldDelay) call and always call that method with a small change inside that method:

  • if preemption is running the start time will be cleaned up automatically
  • if preemption is not running and delay is not set now clear out the start time
  • all other cases let it flow as it was

Removes the need to track quotaPreemptionDelaySet

}

// GetQueuePath returns the fully qualified path of this queue.
Expand Down
42 changes: 42 additions & 0 deletions pkg/scheduler/objects/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3263,6 +3263,48 @@ func TestQueueBackoffProperties(t *testing.T) {
assert.Equal(t, 30*time.Second, leaf3.GetBackoffDelay())
}

func TestUpdateQueuePropertiesReset(t *testing.T) {
root, err := createRootQueue(nil)
assert.NilError(t, err, "failed to create basic root queue")

leaf, err := createManagedQueue(root, "leaf", false, nil)
assert.NilError(t, err, "failed to create managed queue")

leaf.properties = map[string]string{
configs.ApplicationSortPolicy: policies.FairSortPolicy.String(),
configs.ApplicationSortPriority: configs.ApplicationSortPriorityDisabled,
configs.PriorityOffset: "5",
configs.PriorityPolicy: policies.FencePriorityPolicy.String(),
configs.PreemptionDelay: "10s",
configs.PreemptionPolicy: policies.FencePreemptionPolicy.String(),
configs.ApplicationUnschedulableAsksBackoff: "12",
configs.ApplicationUnschedulableAsksBackoffDelay: "20s",
configs.QuotaPreemptionDelay: "1m",
}
leaf.UpdateQueueProperties(nil)
assert.Equal(t, leaf.sortType, policies.FairSortPolicy)
assert.Equal(t, leaf.prioritySortEnabled, false)
assert.Equal(t, leaf.priorityOffset, int32(5))
assert.Equal(t, leaf.priorityPolicy, policies.FencePriorityPolicy)
assert.Equal(t, leaf.preemptionDelay, 10*time.Second)
assert.Equal(t, leaf.preemptionPolicy, policies.FencePreemptionPolicy)
assert.Equal(t, leaf.unschedAskBackoff, uint64(12))
assert.Equal(t, leaf.askBackoffDelay, 20*time.Second)
assert.Equal(t, leaf.quotaPreemptionDelay, time.Minute)

leaf.properties = map[string]string{}
leaf.UpdateQueueProperties(nil)
assert.Equal(t, leaf.sortType, policies.FifoSortPolicy)
assert.Equal(t, leaf.prioritySortEnabled, true)
assert.Equal(t, leaf.priorityOffset, int32(0))
assert.Equal(t, leaf.priorityPolicy, policies.DefaultPriorityPolicy)
assert.Equal(t, leaf.preemptionDelay, configs.DefaultPreemptionDelay)
assert.Equal(t, leaf.preemptionPolicy, policies.DefaultPreemptionPolicy)
assert.Equal(t, leaf.unschedAskBackoff, uint64(0))
assert.Equal(t, leaf.askBackoffDelay, configs.DefaultAskBackOffDelay)
assert.Equal(t, leaf.quotaPreemptionDelay, configs.DefaultQuotaPreemptionDelay)
}

func TestQueue_setPreemptionTime(t *testing.T) {
root, e := createRootQueue(nil)
assert.NilError(t, e, "failed to create basic root queue")
Expand Down
Loading