KEP-1282: Requeue Strategy

[nstogner]

What type of PR is this?

/kind documentation

What this PR does / why we need it:

Document how we could add support for configurable queue ordering strategies upon eviction.

Which issue(s) this PR fixes:

Part of #1282

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	dc9c32e
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/65824a42916aaa00087887ac

[k8s-ci-robot]

Hi @nstogner. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[mimowo]

/ok-to-test
/assign

[mimowo]

Is't it the other way round? UseEvictionTimestamp -> front of the queue?

[nstogner]

I meant UseEvictionTimestamp as the time in which to consider for entry to queue. Because the eviction would have occurred more recently than the initial .metadata.creationTimestamp, this would put the Workload at the "back of the queue" immediately following eviction. If you read this differently, perhaps it would be good to change the terminology.

[alculquicondor]

/release-note-none

[alculquicondor]

It looks like my concern was unfounded.
We handle this problem here:

kueue/pkg/queue/cluster_queue_best_effort_fifo.go

Line 43 in c2f21f4

return cq.requeueIfNotPresent(wInfo, reason == RequeueReasonFailedAfterNomination || reason == RequeueReasonPendingPreemption)

Basically, if the workload has preempted any workloads, it's put back in the queue immediately.

Since nothing changes about it and it will have a higher priority than the preempted workloads, it should be at the head of the queue.

[alculquicondor]

We consider multiple aspects for sorting, the timestamp being one of them.

Having UseCreationTimestamp as the name of the option seems to imply that we ignore any other aspects.

Maybe it should be something like:

requeueStrategy:
  podsReadyTimeout:
    timestampSource: Creation | Eviction

That is assuming we are going to offer configuration for every eviction mechanism, otherwise, I would put the setting under the existing waitForPodsReady:

waitForPodsReady:
  requeuingTimestamp: Creation | Eviction

[denkensk]

I'm struggling to grasp how varying eviction methods could result in distinct types of queue sorting. Can you share some cases?

[alculquicondor]

Suggested change

	- Add an integration test to detect if flapping associated with preempted workloads being readmitted before the preemptor workload when `priorityPreemption: UseEvictionTimestamp` is set.
	- Add an integration test to detect if flapping associated with preempted workloads being readmitted before the preemptor workload when `requeuingTimestamp: Creation` is set.

?

[nstogner]

Good catch, updated.

[alculquicondor]

There are other drawbacks, for example, if you want to use waitForPodsReady to prevent jobs with invalid images to block the queue, then you would continuosly block the queue.
Until we have a proper mechanism to fail pods in this situation.
kubernetes/kubernetes#122300

[nstogner]

Good callout, added an bullet point.

[alculquicondor]

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: b0a5586f1b484b10a88ae5316c68c95ca66dce39

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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 [alculquicondor]

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

[tenzen-y]

@alculquicondor @nstogner I apologize for being too late in my comment.
Can we add a knob to set the limit on the number of requeueing?

I guess that infinite requeueing could happen when node resources are balanced (which means not bin-packing).
Not bin-packing means there are enough node resources in the cluster, although there aren't enough node resources in a single node.

[alculquicondor]

What would you like to happen when the threshold is reached? In a way, it seems independent from what place in the queue the workload should have after being requeued.

[tenzen-y]

What would you like to happen when the threshold is reached? In a way, it seems independent from what place in the queue the workload should have after being requeued.

When we use requeuingTimestamp=Creation, I meant the reached threshold job is 1. evicted and abandoned or 2. put on the tail of the queue.

[alculquicondor]

I think it's simpler to abandon the job, but again, it seems like a separate feature.

But if you have a proposal based on a user story, maybe you can send an update to this KEP?

[tenzen-y]

I think it's simpler to abandon the job, but again, it seems like a separate feature.

But if you have a proposal based on a user story, maybe you can send an update to this KEP?

Yes,sure. I can update this KEP.

[alculquicondor]

@tenzen-y I was talking about this with @mwielgus, and we came to the conclusion that it might be better to introduce a form of backoff: for some period of time, the workload doesn't enter the queue. And the backoff could be exponential.

This might be better than reducing priority, as that would make the workload more susceptible to preemptions once admitted.

[tenzen-y]

@tenzen-y I was talking about this with @mwielgus, and we came to the conclusion that it might be better to introduce a form of backoff: for some period of time, the workload doesn't enter the queue. And the backoff could be exponential.

This might be better than reducing priority, as that would make the workload more susceptible to preemptions once admitted.

@alculquicondor Totally SGTM. Will we work on the backoff mechanism in the first iteration?

Also, I still think we should introduce a timeout mechanism, as I said above. For example, we could avoid jobs without proper image credentials staying in the queue forever. WDYT?

[alculquicondor]

Will we work on the backoff mechanism in the first iteration?

I think it's doable. Let me get back to you tomorrow.

Also, I still think we should introduce a timeout mechanism, as I said above. For example, we could avoid jobs without proper image credentials staying in the queue forever. WDYT?

It becomes less important once we have an exponential backoff. Maybe we can add a max number of retries to the backoff, after which the Workload is considered failed.

[tenzen-y]

It becomes less important once we have an exponential backoff. Maybe we can add a max number of retries to the backoff, after which the Workload is considered failed.

That makes sense. I prefer a max number of retries to the backoff rather than timeout.

[tenzen-y]

Will we work on the backoff mechanism in the first iteration?

I think it's doable. Let me get back to you tomorrow.

@alculquicondor Is there any progress about iterations for the backoff mechanism?

[alculquicondor]

@nstogner will first work on what is already designed and then he will work on a backoff mechanism. Hopefully we can have both for the release.

If you have some time to finalize the design, feel free to open a PR.

[tenzen-y]

@nstogner will first work on what is already designed and then he will work on a backoff mechanism. Hopefully we can have both for the release.

If you have some time to finalize the design, feel free to open a PR.

That makes sense. Thanks!

[tenzen-y]

I will update this KEP for the backoff mechanism.