Fix using PrioritySortingWithinCohort=false and borrowWithinCohort

[mimowo]

What type of PR is this?

What this PR does / why we need it:

To prevent infinite preemption loop when borrowWithinCohort is used with PrioritySortingWithingCohort=false

Which issue(s) this PR fixes:

Fixes #2821

Special notes for your reviewer:

In the scenario described in #2821 when PrioritySortingWithingCohort=false we re-admit repeatedly workload-A (lower priority) which blocks the admission of the Workload-B2 (higher priority).

Ideally, we would like workload-B to be scheduled, which would break the cycle. Workload-B2 would not get preempted by Workload-A.

The code that is important for relative prioritization of the two workloads is here:

1. [L630-634] both Workload-A and Workload-B2 need to borrow to get admitted
2. [L637-639] fair sharing is not configured here, so this block is not relevant
3. [L641-648] PrioritySortingWithingCohort=false so this block is not relevant
4. [L651-653] Workload-A currently "wins" here because it is older

The idea for the fix in to modify the formula for GetQueueOrderTimestamp at the last step and use the timestamp to discriminate in favor of Workload-B2. In this order we use the EvictionTimestamp rather than creation timestamp. This idea is not entirely new as we do something similar to penalize workload evicted due to PodsReady timeout already.

Does this PR introduce a user-facing change?

Prevent infinite preemption loop when PrioritySortingWithinCohort=false
is used together with borrowWithinCohort.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

• OWNERS [mimowo]

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

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	f09db5f
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66ba3866d863b70008cc8c64

[mimowo]

/test pull-kueue-test-integration-main

[mimowo]

/assign @gabesaba

[mimowo]

/cc @PBundyra

[PBundyra]

Could you please describe the mechanism in description behind preventing the infinite loop? Especially which workload does Kueue should admit after the changes

[PBundyra]

How do we estimate that 1ms will be enough to put the preemptor in front of the workload being preemtepted?
Other than that, LGTM

[mimowo]

Could you please describe the mechanism in description behind preventing the infinite loop? Especially which workload does Kueue should admit after the changes

Good idea, I have updated the "Special notes for your reviewer:" section. PTAL.

[mimowo]

How do we estimate that 1ms will be enough to put the preemptor in front of the workload being preemtepted?
Other than that, LGTM

Yes, any epsilon would be enough. This is just to discriminate them in scenario when the creation timestamp of the preemptor equals eviction timestamp of the preempted workload. This is a practical possibility which was discovered by the integration test when I was running it in a loop. It would not be end of the world - the preempted workload-A in practice would get admitted and preempted again. Eventually its eviction timestamp would be greater than the creation timestamp of Workload-B2 (but it could take a couple of attempts as the resolution of timestamps is 1s).

[PBundyra]

Thank you, LGTM

[gabesaba]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 39f12fcb2958cf47566f8609c908a9c6c828a1e7

[mimowo]

/cherry-pick release-0.8

[k8s-infra-cherrypick-robot]

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

In response to this:

/cherry-pick release-0.8

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]

@mimowo: new pull request created: #2830

In response to this:

/cherry-pick release-0.8

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.

[alculquicondor]

/release-note-edit

Prevent infinite preemption loop when PrioritySortingWithinCohort=false
is used together with borrowWithinCohort.

[mimowo]

/kind bug