Skip to content

Conversation

@gabesaba
Copy link
Contributor

@gabesaba gabesaba commented Nov 15, 2024

What type of PR is this?

/kind bug

What this PR does / why we need it:

We fix the calculation of borrowing in FlavorAssignment for Preemption fit mode. Now, it matches how we calculate borrowing for Fit mode. This ensures that a workload which fits within its CQ's nominal capacity is sorted before an earlier created workload which pushes its CQ over nominal capacity. This sorting is important as this borrowing workload may invalidate the non-borrowing workload's preemption calculations

Which issue(s) this PR fixes:

Fixes #3405

Special notes for your reviewer:

We tweak an integ test which relied on details of FlavorAssigner's borrowing calculations for ordering. By updating the priority of pendingWl, we ensure that it reservers resources and prevents blockedWl from scheduling.

Does this PR introduce a user-facing change?

Determine borrowing more accurately, allowing preempting workloads which fit in nominal quota to schedule faster

@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/bug Categorizes issue or PR as related to a bug. labels Nov 15, 2024
@k8s-ci-robot k8s-ci-robot requested review from mimowo and trasc November 15, 2024 14:13
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 15, 2024
@netlify
Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 25d59d5
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/673771af57da550008030873

@gabesaba gabesaba force-pushed the prioritize_nominal_wl branch 3 times, most recently from 2e78f0a to cac6df1 Compare November 15, 2024 16:00
@gabesaba gabesaba force-pushed the prioritize_nominal_wl branch from cac6df1 to 25d59d5 Compare November 15, 2024 16:07
@mimowo
Copy link
Contributor

mimowo commented Nov 15, 2024

I synced with @gabesaba and did review on a call. The main changes since the initial version were:

  • added && a.cq.HasParent() to borrow := a.cq.BorrowingWith(fr, val) && a.cq.HasParent()
  • improve readability of the new test case scenario by matching the workload names to the issue description

Hope this helps, feel free to ask more question in comments if you feel so.

/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 Nov 15, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabesaba, mimowo

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: ac0f7b4cf0e067a2d008edbd4b25b6cdf4b2232d

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2024
@mimowo
Copy link
Contributor

mimowo commented Nov 15, 2024

We are going to cherry-pick this PR along with #3536

@k8s-ci-robot k8s-ci-robot merged commit 98ec495 into kubernetes-sigs:main Nov 15, 2024
17 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.10 milestone Nov 15, 2024
@mimowo
Copy link
Contributor

mimowo commented Nov 15, 2024

/cherry-pick release-0.9

@k8s-infra-cherrypick-robot
Copy link
Contributor

@mimowo: new pull request created: #3550

In response to this:

/cherry-pick release-0.9

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-sigs/prow repository.

gabesaba added a commit to gabesaba/kueue that referenced this pull request Nov 15, 2024
@gabesaba gabesaba deleted the prioritize_nominal_wl branch November 15, 2024 16:56
k8s-ci-robot pushed a commit that referenced this pull request Nov 15, 2024
* cherry-pick #3536

* cherry-pick #3547
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/bug Categorizes issue or PR as related to a bug. 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workload above nominal of CQ prioritized over workload within nominal quota of other CQ

4 participants