-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KEP: time dependent job priority #884
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue canceled.
|
Hi @Gekko0114. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Gekko0114 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@alculquicondor, @kerthcet, @tenzen-y, @trasc, Thank you so much for your discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/assign @mwielgus
I will check after the release of v0.4. |
This proposal allows job priorities to be dynamically adjusted based on time. | ||
This functionality aims to prioritize the execution of jobs that have been waiting | ||
for an extended period, even if they have a lower initial priority. | ||
By introducing time-dependent priority adjustments, the system can ensure that jobs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about introducing just a priority/ordering annotation and keeping the updates outside of Kueue, in a simple controller? I can imagine quite a few different use cases for custom ordering in a queue and obviously not all of them can be added to the API and implemented.
### Other points | ||
If a Job has both the `spec.PriorityClass` and WorkloadPriorityClass defined, `spec.PriorityClass` is ignored. | ||
Since the `creationTimeStamp` of the Job is always available, the calculation can be performed every time | ||
without updating the values of Job and Workload Priority. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can make troubleshooting harder. In case of a mix of jobs with different workload priority class it will be impossible to tell what the current order of workloads actually is at the moment (without copying them to a spreadsheet or running some additional custom scripts).
/ok-to-test |
|
||
} | ||
|
||
type DynamicPriorityPolicy struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can quickly become either insufficient or very cluttered. For example, I may want to have this logic outside of business hours and on weekends. Plus maybe deprioritise workloads that have been waiting for too long as their results may not actually be that relevant anymore. And so on.
Hi @alculquicondor, @mwielgus, Thanks for your comment. I've seen the proposal for #973. Should we prioritize that proposal over this KEP? |
@alculquicondor , |
I will close this PR since workloadPriorityClass was merged |
What type of PR is this?
/kind documentation
/kind api-change
What this PR does / why we need it:
KEP for #754
Special notes for your reviewer:
Thanks for your review.
Does this PR introduce a user-facing change?