Graduate PrioritySortingWithinCohort feature gate to GA#9259
Graduate PrioritySortingWithinCohort feature gate to GA#9259kannon92 wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Set PrioritySortingWithinCohort to GA with LockToDefault: true at version 0.17. Remove all conditional code paths guarded by the feature gate since priority sorting within cohorts is now always enabled. Remove associated disabled-feature test cases and update documentation to no longer reference the ability to disable the feature.
26c0127 to
af7aa8e
Compare
|
/lgtm Thank you! |
|
LGTM label has been added. DetailsGit tree hash: 3aa4431baddb40424fdc55650d9098b7111867b1 |
tenzen-y
left a comment
There was a problem hiding this comment.
I can approve this GA graduation, but I think that we should recommend enabling FS (for preemption enabling users) or AFS (for preemption disabling users) in the release note.
As far as I can remember, this FG, we introduced this mechanism with FG since we want to rapidly ship this one.
#1283
Additionally, we discussed adding this setting to the Kueue Configuration rather than using FG gating because this enhancement allows evil users to prioritize their workloads within Cohort.
After some discussions, we introduced FS and AFS to mitigate such evil user behavior.
+1 it will be better to say that users who disable IIRC @amy used to use that, but AFAIK no longer uses it.
Yes, but given we have FS I don't see a need to maintain this FG. |
Yes, I agree with you. For now, we can just recommend FS / AFS. If we can get strong motivation for the disabling of that, we can revisit the dedicated knob. |
|
No change needed. Let's wait a week or so with the PR in case some community members have objections. In particular, I would like to hear from @amy to confirm she successfully managed to stop using the FG. If no objections raised we can move the PR forward. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kannon92, mimowo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I agree with you. Let's bring this topic to the next wg-batch meeting. |
I added it to agenda.
|
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Graduates the
PrioritySortingWithinCohortfeature gate to GA in version 0.17, withLockToDefault: true.This feature has been enabled by default since v0.6 (Beta). Priority sorting within cohorts is now permanently enabled and can no longer be disabled.
Changes:
LockToDefault: true(to be removed in 0.19)scheduler.go,fair_sharing_iterator.go, andworkload.goGetQueueOrderTimestampthat handled the disabled case with special preemption timestamp logic"When borrowWithinCohort is used and PrioritySortingWithinCohort disabled"TestEntryOrderingwhereprioritySorting: falseversioned_feature_list.yamlin both site and test referenceWhich issue(s) this PR fixes:
Part of #8855
Special notes for your reviewer:
The feature has been Beta and enabled by default since v0.6, so this graduation follows the standard lifecycle. The
featuresimport was removed fromfair_sharing_iterator.goas it was the only usage in that file.Does this PR introduce a user-facing change?