Skip to content
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

Graduate MultiplePreemptions to Beta #2864

Merged

Conversation

macsko
Copy link
Contributor

@macsko macsko commented Aug 20, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

Graduate MultiplePreemptions feature to beta and enable it by default.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Graduated MultiplePreemptions to Beta and enabled by default.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 20, 2024
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 20, 2024
@macsko
Copy link
Contributor Author

macsko commented Aug 20, 2024

/cc @alculquicondor @gabesaba

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 20, 2024
Copy link

netlify bot commented Aug 20, 2024

Deploy Preview for kubernetes-sigs-kueue ready!

Name Link
🔨 Latest commit 61ebacd
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66c4971ad5aaaf000895f485
😎 Deploy Preview https://deploy-preview-2864--kubernetes-sigs-kueue.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@alculquicondor
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, macsko

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: af59462725131c035c2202d2b87926370f2a2257

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2024
@k8s-ci-robot k8s-ci-robot merged commit 30ae016 into kubernetes-sigs:main Aug 20, 2024
16 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.9 milestone Aug 20, 2024
@danielezhu
Copy link

Hi, I couldn't find any explicit mention of MultiplePreemptions in the API reference docs or the Concepts/Preemption section and was wondering if someone could please explain what enabling this feature does.

Based on its name, I thought that enabling it allows high-priority workloads to preempt multiple workloads (in other words, by default, workloads can only preempt at most 1 workload), but based on testing a toy example, it seems that high-priority workloads can already do this (even without the MultiplePreemptions feature gate enabled).

Test:

  1. Create ClusterQueue with a nominalQuota of 8 cpus
  2. Create two low-priority jobs that request 4 cpus each
  3. Create a high-priority job that requests 8 cpus

What I observed is that after step 2, both low-priority jobs were running. Upon submitting the high-priority job, both the low-priority jobs became pending and the high-priority job got admitted.

@danielezhu
Copy link

Ah, I just realized this has already been merged.

@tenzen-y
Copy link
Member

Ah, I just realized this has already been merged.

Yeah, that's true. After we release v0.9.0, this feature is enabled by default, and we can see the details in the website.

@alculquicondor
Copy link
Contributor

We didn't document it because it was meant to become the default behavior and it might not be a very common scenario that users run into. But you can find more details in the original PR #2641

@danielezhu
Copy link

danielezhu commented Oct 16, 2024

Ah, I just realized this has already been merged.

Yeah, that's true. After we release v0.9.0, this feature is enabled by default, and we can see the details in the website.

Just to confirm, this feature is also enabled by default if I'm using v0.8.1, right? Since this PR got merged prior to the 0.8.1 release. Hence why my simple test case allowed both low-prio workloads to get preempted (I'm using 0.8.1).

@alculquicondor
Copy link
Contributor

The feature will only be enabled by default in v0.9. Your test case has always worked.

This feature is about multiple preemptions from multiple ClusterQueues in a cohort. It always faster throughput of preemptions when pods take a long time to finish.

@danielezhu
Copy link

The feature will only be enabled by default in v0.9. Your test case has always worked.

This feature is about multiple preemptions from multiple ClusterQueues in a cohort. It always faster throughput of preemptions when pods take a long time to finish.

Thanks for your clarification. The MultiplePreemptions feature is clear to me now; I didn't read the PR description carefully enough at first. Thank you and @tenzen-y very much for your prompt replies and clear explanations!

kannon92 pushed a commit to openshift-kannon92/kubernetes-sigs-kueue that referenced this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants