[BestEffortFIFO] Implement Sticky ClusterQueue Head Policy #7157

gabesaba · 2025-10-03T12:37:57Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

When a workload, in a queue with BestEffortFIFO enabled, has preemption targets, we keep it at the head of the queue until it schedules (or returns inadmissible).

See original proposal here: #6929 (comment)

Which issue(s) this PR fixes:

Fixes #7101, #6929

Special notes for your reviewer:

Let's discuss whether we want to put this behind a feature gate.

Does this PR introduce a user-facing change?

Scheduling: With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as
it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible
workload went back to head of queue, in front of the preempting workload, allowing
preempted workloads to reschedule

netlify · 2025-10-03T12:38:04Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`0ce0aec`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/68e51747b0f5980008c9a55b

k8s-ci-robot · 2025-10-03T12:38:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabesaba

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:

~~OWNERS~~ [gabesaba]

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

pkg/cache/queue/cluster_queue.go

mimowo · 2025-10-03T12:56:15Z

Please use the release-note block so that the label do-not-merge/release-note-label-needed is removed by the robot

gabesaba · 2025-10-03T13:27:14Z

Please use the release-note block so that the label do-not-merge/release-note-label-needed is removed by the robot

I think it was because I had BestEffortFIFO in the middle of the block when I created the PR

/release-note-edit

With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as
it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible
workload went back to head of queue, in front of the preempting workload, allowing
preempted workloads to reschedule

k8s-ci-robot · 2025-10-03T13:27:17Z

@gabesaba: /release-note-edit must be used with a single release note block.

In response to this:

Please use the release-note block so that the label do-not-merge/release-note-label-needed is removed by the robot

I think it was because I had BestEffortFIFO in the middle of the block when I created the PR

/release-note-edit
With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as
it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible
workload went back to head of queue, in front of the preempting workload, allowing
preempted workloads to reschedule

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 · 2025-10-03T13:28:42Z

/release-note-edit

With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as
it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible
workload went back to head of queue, in front of the preempting workload, allowing
preempted workloads to reschedule

k8s-ci-robot · 2025-10-03T13:28:45Z

@gabesaba: /release-note-edit must be used with a single release note block.

In response to this:

/release-note-edit

With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as
it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible
workload went back to head of queue, in front of the preempting workload, allowing
preempted workloads to reschedule

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 · 2025-10-03T13:29:42Z

/release-note-edit

With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible workload went back to head of queue, in front of the preempting workload, allowing preempted workloads to reschedule

k8s-ci-robot · 2025-10-03T13:29:45Z

@gabesaba: /release-note-edit must be used with a single release note block.

In response to this:

/release-note-edit

With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible workload went back to head of queue, in front of the preempting workload, allowing preempted workloads to reschedule

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.

pkg/cache/queue/cluster_queue.go

test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go

mimowo · 2025-10-03T14:31:07Z

test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go

+			for range 4 {
+				createWorkload("cq-p1", "2")
+			}
+			util.ExpectReservingActiveWorkloadsMetric(cqp1, 4)


nit, maybe it is better to use the Admitted metrics as this could sometimes be updated before the quota reservation is fully processed, causing flakes, see: #7155

mimowo · 2025-10-03T14:37:44Z

/lgtm
/cherrypick release-0.14
/cherrypick release-0.13

In case you could fix #7157 (comment). Since this is just a potential issue I'm ok to optimize for fix delivery, feel free to unhold, we could potentially fix in a follow up.
/hold

k8s-infra-cherrypick-robot · 2025-10-03T14:37:47Z

@mimowo: once the present PR merges, I will cherry-pick it on top of release-0.13, release-0.14 in new PRs and assign them to you.

In response to this:

/lgtm
/cherrypick release-0.14
/cherrypick release-0.13

In case you could fix #7157 (comment). Since this is just a potential issue I'm ok to optimize, feel free to unhold, we could potentially fix in a follow up.
/hold

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.

mimowo · 2025-10-03T17:14:27Z

test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go

+				util.FinishEvictionOfWorkloadsInCQ(ctx, k8sClient, cq2, 2)
+
+				ginkgo.By("Expect active workloads")
+				util.ExpectReservingActiveWorkloadsMetric(cq1, 1)


I tried running locally and hit a flake here on the first run:

[FAILED] Timed out after 10.000s. The function passed to Eventually failed at /usr/local/google/home/michalwozniak/go/src/sigs.k8s.io/kueue/test/util/util.go:571 with: Expected <int>: 0 to equal <int>: 1 In [It] at: /usr/local/google/home/michalwozniak/go/src/sigs.k8s.io/kueue/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go:898 @ 10/03/25 17:11:51.661

PTAL

This flake seems to go away when is addressed: #7157 (comment)

I ran this test 32 times without flakes. I'll try running 128 times now to see if there are any.

I see, maybe this is fixed by the metric change? I will also test locally with the metric change

Indeed, I tested locally 60 times after the latest changes and no failures, feel free to resolve.

It flaked 3/128 times for me.

There is a race condition where the preempted workloads schedule, given that we consider the inadmissible head after deleting the sticky workload

I updated the test to keep finishing the evictions. It passed 128/128 runs:

gomega.Eventually(func(g gomega.Gomega) { util.FinishEvictionsOfAnyWorkloadsInCq(ctx, k8sClient, cq2) util.ExpectAdmittedWorkloadsTotalMetric(cq1, "", 1) util.ExpectClusterQueueWeightedShareMetric(cq1, 0) util.ExpectClusterQueueWeightedShareMetric(cq2, 0) }, util.Timeout, util.Interval).Should(gomega.Succeed())

mimowo · 2025-10-07T13:44:32Z

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/7157/pull-kueue-test-integration-extended-main/1975555739745259520
Does not look related, I will open a flake issue

EDIT: #7194

mimowo · 2025-10-07T13:59:42Z

/test pull-kueue-test-integration-extended-main

mimowo · 2025-10-07T15:03:01Z

Thank you 👍

/lgtm
/cherrypick release-0.14
/cherrypick release-0.13

k8s-infra-cherrypick-robot · 2025-10-07T15:03:04Z

@mimowo: once the present PR merges, I will cherry-pick it on top of release-0.13, release-0.14 in new PRs and assign them to you.

In response to this:

Thank you 👍

/lgtm
/cherrypick release-0.14
/cherrypick release-0.13

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.

k8s-ci-robot · 2025-10-07T15:03:10Z

LGTM label has been added.

Git tree hash: 01585ba9d560c7214e5fd48ab4bfb68291ff7477

k8s-infra-cherrypick-robot · 2025-10-07T15:05:43Z

@mimowo: new pull request created: #7197

In response to this:

Thank you 👍

/lgtm
/cherrypick release-0.14
/cherrypick release-0.13

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.

k8s-infra-cherrypick-robot · 2025-10-07T15:06:18Z

@mimowo: #7157 failed to apply on top of branch "release-0.13":

Applying: Implement Sticky ClusterQueue Head Policy
Using index info to reconstruct a base tree...
A	pkg/cache/queue/cluster_queue.go
M	test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go
Falling back to patching base and 3-way merge...
Auto-merging test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go
Auto-merging pkg/queue/cluster_queue.go
CONFLICT (content): Merge conflict in pkg/queue/cluster_queue.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Implement Sticky ClusterQueue Head Policy

In response to this:

Thank you 👍

/lgtm
/cherrypick release-0.14
/cherrypick release-0.13

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.

tenzen-y · 2025-10-08T06:29:43Z

@gabesaba Can you manually open cherry-pick PR to release-0.13?

gabesaba · 2025-10-08T07:46:26Z

@gabesaba Can you manually open cherry-pick PR to release-0.13?

Yes. I am waiting for one more PR that I want to cherry-pick at the same time: additional logging for this feature.

tenzen-y · 2025-10-08T07:59:38Z

@gabesaba Can you manually open cherry-pick PR to release-0.13?

Yes. I am waiting for one more PR that I want to cherry-pick at the same time: additional logging for this feature.

Which PR do you indicate? Has it already merged?

…s-sigs#7157) * [BestEffortFIFO] Implement Sticky ClusterQueue Head Policy * assert using ExpectAdmittedWorkloadsTotalMetric * FinishEvictionOfAnyWorkloadsInCq method * fix flakiness; new test case

mimowo · 2025-10-08T08:11:53Z

Here: #7201

It will get cherrypicked to 0.14 without conflicts.

We can already start preparing the cherrypick for 0.13 manually, and open the PR for them.

* [BestEffortFIFO] Implement Sticky ClusterQueue Head Policy (#7157) * [BestEffortFIFO] Implement Sticky ClusterQueue Head Policy * assert using ExpectAdmittedWorkloadsTotalMetric * FinishEvictionOfAnyWorkloadsInCq method * fix flakiness; new test case * logging for 7157

mimowo · 2025-11-28T08:53:49Z

/release-note-edit

Scheduling: With BestEffortFIFO enabled, we will keep attempting to schedule a workload as long as
it is waiting for preemption targets to complete. This fixes a bugs where an inadmissible
workload went back to head of queue, in front of the preempting workload, allowing
preempted workloads to reschedule

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 3, 2025

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 3, 2025

k8s-ci-robot requested a review from kannon92 October 3, 2025 12:38

k8s-ci-robot requested a review from PBundyra October 3, 2025 12:38

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 3, 2025

gabesaba force-pushed the sticky branch from 7ed241a to e5a7e6c Compare October 3, 2025 12:42

mimowo reviewed Oct 3, 2025

View reviewed changes

pkg/cache/queue/cluster_queue.go Outdated Show resolved Hide resolved

PBundyra reviewed Oct 3, 2025

View reviewed changes

pkg/cache/queue/cluster_queue.go Show resolved Hide resolved

test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go Show resolved Hide resolved

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 3, 2025

[BestEffortFIFO] Implement Sticky ClusterQueue Head Policy

0683350

gabesaba force-pushed the sticky branch from e5a7e6c to 0683350 Compare October 3, 2025 14:07

mimowo reviewed Oct 3, 2025

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2025

k8s-ci-robot assigned mimowo Oct 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2025

mimowo reviewed Oct 3, 2025

View reviewed changes

gabesaba force-pushed the sticky branch from b13c93f to 0ce0aec Compare October 7, 2025 13:36

mimowo mentioned this pull request Oct 7, 2025

add admin and user roles for trainjob to kueue rbac #7196

Merged

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2025

k8s-ci-robot merged commit f8d2887 into kubernetes-sigs:main Oct 7, 2025
23 checks passed

k8s-ci-robot added this to the v0.15 milestone Oct 7, 2025

k8s-infra-cherrypick-robot mentioned this pull request Oct 7, 2025

[release-0.14] [BestEffortFIFO] Implement Sticky ClusterQueue Head Policy #7197

Merged

gabesaba deleted the sticky branch October 7, 2025 15:25

gabesaba mentioned this pull request Oct 8, 2025

logging for 7157 #7201

Merged

gabesaba mentioned this pull request Oct 8, 2025

[release-0.13] Cherry-pick #7157, #7201 #7202

Merged

gabesaba mentioned this pull request Oct 8, 2025

Workload scheduling failure when preemption (reclaimWithinCohort) is required and cluster queue head has not-schedulable workload(s). #6929

Closed

amy mentioned this pull request Oct 13, 2025

Introduce a scheduling category for workloads with more permanent failures #7252

Open

3 tasks

ichekrygin mentioned this pull request Oct 17, 2025

Higher-priority workloads are blocked while a “sticky” low-priority workload is pinned to the head of the queue #7301

Open

mwysokin mentioned this pull request Nov 6, 2025

Fail to preempt an older low-priority job when borrowing within cohort #6814

Open

gabesaba mentioned this pull request Nov 14, 2025

Log Sticky Workload Deletion Path #7654

Merged

This was referenced Nov 14, 2025

Kueue Scheduler getting stuck if preemption request fails #5590

Closed

Better test coverage for scheduler handling of failed preemptions #7806

Open

[BestEffortFIFO] Implement Sticky ClusterQueue Head Policy #7157

[BestEffortFIFO] Implement Sticky ClusterQueue Head Policy #7157

Uh oh!

Conversation

gabesaba commented Oct 3, 2025 • edited by k8s-ci-robot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

netlify bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

Uh oh!

mimowo commented Oct 3, 2025

Uh oh!

gabesaba commented Oct 3, 2025

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

gabesaba commented Oct 3, 2025

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

gabesaba commented Oct 3, 2025

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimowo commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-infra-cherrypick-robot commented Oct 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimowo commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mimowo commented Oct 7, 2025

Uh oh!

mimowo commented Oct 7, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Oct 7, 2025

Uh oh!

k8s-ci-robot commented Oct 7, 2025

Uh oh!

Uh oh!

k8s-infra-cherrypick-robot commented Oct 7, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Oct 7, 2025

Uh oh!

tenzen-y commented Oct 8, 2025

Uh oh!

gabesaba commented Oct 8, 2025

Uh oh!

gabesaba commented Oct 3, 2025 •

edited by k8s-ci-robot

Loading

netlify bot commented Oct 3, 2025 •

edited

Loading

mimowo commented Oct 3, 2025 •

edited

Loading

mimowo commented Oct 7, 2025 •

edited

Loading