Delete `assumedWorkloads` field from cache by PBundyra · Pull Request #8474 · kubernetes-sigs/kueue

PBundyra · 2026-01-08T15:52:40Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

After code analysis and experimenting what happens if we deleted assumedWorkloads I came to conclusion that it is redundant. Kueue doesn't leverage information about Workload being assumed, and it doesn't differ from just adding it to cache. AssumeWorkloads eventually adds it to the cache and the workload is treated as it was just added (not assumed). Hence this PR to simplify the code and maintenance

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

netlify · 2026-01-08T15:52:48Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`3e3af50`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6964f0b0488bb40008c1d48e

PBundyra · 2026-01-09T09:27:44Z

cc @mimowo @Singularity23x0

…st like scheduler

mimowo · 2026-01-12T11:01:42Z

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/8474/pull-kueue-test-integration-baseline-main/2010655165719252992
haven't seen tests failing like this before, @PBundyra please check if this could be related to the PR. Let me re-trigger the tests to see if this is a flake or stable failing

mimowo · 2026-01-12T11:01:51Z

/test pull-kueue-test-integration-baseline-main

PBundyra · 2026-01-12T11:04:59Z

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/8474/pull-kueue-test-integration-baseline-main/2010655165719252992 haven't seen tests failing like this before, @PBundyra please check if this could be related to the PR. Let me re-trigger the tests to see if this is a flake or stable failing

I'm looking into it

PBundyra · 2026-01-12T13:03:01Z

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/8474/pull-kueue-test-integration-baseline-main/2010655165719252992 haven't seen tests failing like this before, @PBundyra please check if this could be related to the PR. Let me re-trigger the tests to see if this is a flake or stable failing

It seems like the new added test was causing the problem. There I introduced a variable numCalls that was changed by the Patch call in scheduler, and read in the test at the same time which led to data race. Switch to atomic int to prevent race condition

mimowo · 2026-01-12T13:30:53Z

Ah, I see. We could probably just call ginkgo.Fail from within the fakeSubResourcePatchSpec, but this is more generic, so let's go with that

mimowo · 2026-01-12T13:35:08Z

 	return false
 }

-func (c *Cache) AssumeWorkload(log logr.Logger, w *kueue.Workload) error {


Lovely to see this much code removed and all tests still pass ❤️ . Thank you for investigating 👍

mimowo

/lgtm
/approve
Thank you for the simplification, I couldn't see any problematic cases except for the error handling, but the new integration test shows it works well. Let's go with that,

k8s-ci-robot · 2026-01-12T13:40:11Z

LGTM label has been added.

Details

Git tree hash: bcd41dd3f2df4f5f6c147b6c0581e292a5ecfd02

k8s-ci-robot · 2026-01-12T13:40:12Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [PBundyra,mimowo]

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

tenzen-y · 2026-01-12T14:16:44Z

-func (c *Cache) AssumeWorkload(log logr.Logger, w *kueue.Workload) error {
-	c.Lock()
-	defer c.Unlock()


The "assume mechanism" could potentially contribute to avoiding lock-acquiring deadlock between controllers and the scheduler, which can improve kueue-controller-manager performance.

OTOH, AssumeWorkload takes an entire lock, as you can see here. So the current "assume mechanism" is no longer meaningful, I believe.

We can revisit the "assume mechanism" once we observe the cache lock-acquiring error many times.

@PBundyra Thank you 👍

* Comment assumed workloads in cache * Modify unit tests so they dont call AssumeWorkload but AddOrUpdate just like scheduler * Delete assumed workloads field, add an integration test * Fix verify lint * Cleanup ForgetWorkload func * Fix added test * Fix added test * Synchronize the numCalls counter in tests

k8s-ci-robot requested review from mbobrovskyi and tenzen-y January 8, 2026 15:52

PBundyra force-pushed the rm-assumed-workloads branch from 711fd72 to af3b85f Compare January 9, 2026 13:50

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. 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 Jan 9, 2026

PBundyra changed the title ~~[WIP] Remove assumed workloads from cache~~ Delete assumedWorkloads field from cache Jan 9, 2026

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 9, 2026

PBundyra mentioned this pull request Jan 9, 2026

Added workloadAssignedQueues logic handling it. #8433

Merged

PBundyra force-pushed the rm-assumed-workloads branch from af3b85f to b421657 Compare January 9, 2026 14:00

mimowo reviewed Jan 9, 2026

View reviewed changes

Comment thread test/integration/singlecluster/scheduler/scheduler_test.go Outdated

PBundyra force-pushed the rm-assumed-workloads branch from 91e395d to a31e1d7 Compare January 12, 2026 09:04

Singularity23x0 reviewed Jan 12, 2026

View reviewed changes

Comment thread pkg/cache/scheduler/cache.go Outdated

PBundyra added 4 commits January 12, 2026 10:02

Comment assumed workloads in cache

3e1eb0d

Modify unit tests so they dont call AssumeWorkload but AddOrUpdate ju…

2c2ba84

…st like scheduler

Delete assumed workloads field, add an integration test

d18713f

Fix verify lint

247bbcc

PBundyra force-pushed the rm-assumed-workloads branch from a31e1d7 to 5446f3f Compare January 12, 2026 10:08

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 12, 2026

Cleanup ForgetWorkload func

2fcfe96

Fix added test

71d0278

PBundyra force-pushed the rm-assumed-workloads branch from 5446f3f to 71d0278 Compare January 12, 2026 11:17

PBundyra added 2 commits January 12, 2026 12:11

Fix added test

8b1a442

Synchronize the numCalls counter in tests

3e3af50

mimowo reviewed Jan 12, 2026

View reviewed changes

k8s-ci-robot assigned mimowo Jan 12, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 12, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 12, 2026

k8s-ci-robot merged commit 907c8ee into kubernetes-sigs:main Jan 12, 2026
30 checks passed

k8s-ci-robot added this to the v0.16 milestone Jan 12, 2026

tenzen-y reviewed Jan 12, 2026

View reviewed changes

mimowo mentioned this pull request Jan 12, 2026

Fix flaky ClusterQueue deletion by cleaning assumed workloads from cache #8543

Merged

Conversation

PBundyra commented Jan 8, 2026 • edited 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 Jan 8, 2026 • 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!

PBundyra commented Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

mimowo commented Jan 12, 2026

Uh oh!

mimowo commented Jan 12, 2026

Uh oh!

PBundyra commented Jan 12, 2026

Uh oh!

PBundyra commented Jan 12, 2026

Uh oh!

mimowo commented Jan 12, 2026

Uh oh!

mimowo Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

mimowo left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 12, 2026

Uh oh!

k8s-ci-robot commented Jan 12, 2026

Uh oh!

Uh oh!

tenzen-y Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PBundyra commented Jan 8, 2026 •

edited

Loading

netlify Bot commented Jan 8, 2026 •

edited

Loading