Skip to content

Commit

Permalink
Add feature gate for priorities sorting in a cohort
Browse files Browse the repository at this point in the history
  • Loading branch information
yaroslava-serdiuk committed Dec 12, 2023
1 parent e75090a commit e8a5694
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 22 deletions.
18 changes: 13 additions & 5 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ const (
//
// Enables Kueue visibility on demand
VisibilityOnDemand featuregate.Feature = "VisibilityOnDemand"

// owner: @yaroslava-serdiuk
// kep: https://github.com/kubernetes-sigs/kueue/issues/1283
// beta: v0.6
//
// Enable priority sorting within the cohort.
PrioritySortingWithinCohort featuregate.Feature = "PrioritySortingWithinCohort"
)

func init() {
Expand All @@ -75,11 +82,12 @@ func init() {
// Entries are separated from each other with blank lines to avoid sweeping gofmt changes
// when adding or removing one entry.
var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
PartialAdmission: {Default: true, PreRelease: featuregate.Beta},
QueueVisibility: {Default: false, PreRelease: featuregate.Alpha},
FlavorFungibility: {Default: true, PreRelease: featuregate.Beta},
ProvisioningACC: {Default: false, PreRelease: featuregate.Alpha},
VisibilityOnDemand: {Default: false, PreRelease: featuregate.Alpha},
PartialAdmission: {Default: true, PreRelease: featuregate.Beta},
QueueVisibility: {Default: false, PreRelease: featuregate.Alpha},
FlavorFungibility: {Default: true, PreRelease: featuregate.Beta},
ProvisioningACC: {Default: false, PreRelease: featuregate.Alpha},
VisibilityOnDemand: {Default: false, PreRelease: featuregate.Alpha},
PrioritySortingWithinCohort: {Default: true, PreRelease: featuregate.Beta},
}

func SetFeatureGateDuringTest(tb testing.TB, f featuregate.Feature, value bool) func() {
Expand Down
16 changes: 9 additions & 7 deletions pkg/scheduler/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (s *Scheduler) schedule(ctx context.Context) {
// 3. Calculate requirements (resource flavors, borrowing) for admitting workloads.
entries := s.nominate(ctx, headWorkloads, snapshot)

// 4. Sort entries based on borrowing and timestamps.
// 4. Sort entries based on borrowing, priorities (if enabled) and timestamps.
sort.Sort(entryOrdering(entries))

// 5. Admit entries, ensuring that no more than one workload gets
Expand Down Expand Up @@ -500,14 +500,16 @@ func (e entryOrdering) Less(i, j int) bool {
return !aBorrows
}

// 2. Higher priority first.
p1 := priority.Priority(a.Obj)
p2 := priority.Priority(b.Obj)
if p1 != p2 {
return p1 > p2
// 2. Higher priority first if not disabled.
if features.Enabled(features.PrioritySortingWithinCohort) {
p1 := priority.Priority(a.Obj)
p2 := priority.Priority(b.Obj)
if p1 != p2 {
return p1 > p2
}
}

// 2. FIFO.
// 3. FIFO.
aComparisonTimestamp := workload.GetQueueOrderTimestamp(a.Obj)
bComparisonTimestamp := workload.GetQueueOrderTimestamp(b.Obj)
return aComparisonTimestamp.Before(bComparisonTimestamp)
Expand Down
37 changes: 27 additions & 10 deletions pkg/scheduler/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ func TestEntryOrdering(t *testing.T) {
Info: workload.Info{
Obj: &kueue.Workload{ObjectMeta: metav1.ObjectMeta{
Name: "new_high_pri",
CreationTimestamp: metav1.NewTime(now.Add(3 * time.Second)),
CreationTimestamp: metav1.NewTime(now.Add(4 * time.Second)),
}, Spec: kueue.WorkloadSpec{
Priority: ptr.To[int32](1),
}},
Expand All @@ -1232,7 +1232,7 @@ func TestEntryOrdering(t *testing.T) {
Obj: &kueue.Workload{
ObjectMeta: metav1.ObjectMeta{
Name: "evicted_borrowing",
CreationTimestamp: metav1.NewTime(now),
CreationTimestamp: metav1.NewTime(now.Add(time.Second)),
},
Status: kueue.WorkloadStatus{
Conditions: []metav1.Condition{
Expand Down Expand Up @@ -1273,14 +1273,31 @@ func TestEntryOrdering(t *testing.T) {
},
},
}
sort.Sort(entryOrdering(input))
order := make([]string, len(input))
for i, e := range input {
order[i] = e.Obj.Name
}
wantOrder := []string{"new_high_pri", "old", "recently_evicted", "new", "high_pri_borrowing", "old_borrowing", "evicted_borrowing", "new_borrowing"}
if diff := cmp.Diff(wantOrder, order); diff != "" {
t.Errorf("Unexpected order (-want,+got):\n%s", diff)
for _, tc := range []struct {
name string
prioritySorting bool
wantOrder []string
}{
{
name: "Priority sorting is enabled (default)",
prioritySorting: true,
wantOrder: []string{"new_high_pri", "old", "recently_evicted", "new", "high_pri_borrowing", "old_borrowing", "evicted_borrowing", "new_borrowing"},
},
{
name: "Priority sorting is disabled",
prioritySorting: false,
wantOrder: []string{"old", "recently_evicted", "new", "new_high_pri", "old_borrowing", "evicted_borrowing", "high_pri_borrowing", "new_borrowing"},
},
} {
features.SetFeatureGateDuringTest(t, features.PrioritySortingWithinCohort, tc.prioritySorting)
sort.Sort(entryOrdering(input))
order := make([]string, len(input))
for i, e := range input {
order[i] = e.Obj.Name
}
if diff := cmp.Diff(tc.wantOrder, order); diff != "" {
t.Errorf("%s: Unexpected order (-want,+got):\n%s", tc.name, diff)
}
}
}

Expand Down

0 comments on commit e8a5694

Please sign in to comment.