KEP-5142: Pop pod from backoffQ when activeQ is empty #5144

macsko · 2025-02-06T16:29:18Z

One-line PR description: Add KEP-5142

Issue link: Pop pod from backoffQ when activeQ is empty #5142

Other comments:

k8s-ci-robot · 2025-02-06T16:29:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: macsko
Once this PR has been reviewed and has the lgtm label, please assign huang-wei, jpbetz for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

keps/prod-readiness/OWNERS
keps/sig-scheduling/OWNERS

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

macsko · 2025-02-06T16:35:53Z

/cc @dom4ha @sanposhiho @Huang-Wei @alculquicondor

I haven't finished all the points yet, but the general idea is there.

sanposhiho

Though there're some sections with TODO, looking good overall, aligned with the discussion we had in the issue.

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

sanposhiho · 2025-02-07T00:07:36Z

keps/prod-readiness/sig-scheduling/5142.yaml

@@ -0,0 +1,3 @@
+kep-number: 5142
+alpha:
+  approver: "@wojtek-t"


/cc @wojtek-t

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

sanposhiho

Looking mostly good for me, left only nits.

/cc @alculquicondor @macsko @Huang-Wei

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

sanposhiho · 2025-02-10T15:34:18Z

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/kep.yaml

+approvers:
+  - 
+
+stage: alpha


can we start it from beta per the discussion at the issue? i.e., enable from the day1, but we have a feature gate for a safe guird. wdyt

I think we could do this.

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

k8s-ci-robot · 2025-02-10T15:43:45Z

@sanposhiho: GitHub didn't allow me to request PR reviews from the following users: macsko.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Looking mostly good for me, left only nits.

/cc @alculquicondor @macsko @Huang-Wei

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.

sanposhiho · 2025-02-10T15:44:06Z

I meant @dom4ha

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

dom4ha · 2025-02-11T08:08:21Z

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

+However, in the real world, if the scheduling latency is short enough, there won't be a visible downgrade in throughput.
+This will only happen if there are no pods in activeQ, so this can be mitigated by an appropriate rate of pod creation.
+
+#### Backoff won't be working as natural rate limiter in case of errors


I think we have to distinguish unschedulable from error cases in the first version, otherwise we risk the system overload due to scheduler sending retries storm.

Right, especially if we decide to start from beta directly, it will be implemented in the first version.

dom4ha · 2025-02-11T08:16:10Z

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

+and the backoff time is calculated based on the number of scheduling failures that the pod has experienced.
+If one pod has a smaller attempt counter than others,
+could the scheduler keep popping this pod ahead of other pods because the pod's backoff expires faster than others?
+Actually, that wouldn't happen because the scheduler would increment the attempt counter of pods from backoffQ as well,


I assumed we can leave the logic of incrementing scheduling attempts only after the backoff time expires to avoid rapid increase of the backoff time for pods triggered by large number of events.

It's probably debatable, because the side effect is increasing the chance of pod starvation as described here (pod would get back with it's former backoff timeout, unless it expired).

The approach proposed here would make the backoff more natural, but would cause reporting way higher (in the order of magnitude) values for scheduling attempts.

dom4ha · 2025-02-11T08:24:02Z

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md

+To achieve the goal, activeQ's `pop()` method needs to be changed:
+1. If activeQ is empty, then instead of waiting for a pod to arrive at activeQ, popping from backoffQ is tried.
+2. If backoffQ is empty, then `pop()` is waiting for pod as previously.
+3. If backoffQ is not empty, then the pod is processed like the pod would be taken from activeQ, including increasing attempts number.


increasing attempts number.

It's debatable. It can cause rapid increase in the backoff time for some pods (when many event trigger retry), so I was thinking about computing the backoff based on the original approach to not change the logic too much. In other words, not increase scheduling attempts if all of them happened within the last backoff time window. This way this mechanism would be just a best effort improvement rather than a new approach.

It can cause rapid increase in the backoff time for some pods (when many event trigger retry)

I believe we should increment the attempt number.
It is exactly an intentional behavior to move the pods to backoffQ immediately and make a lot of scheduling retries (regardless of whether with or without this feature), if many events are coming and each triggers the pod's requeueing.
As long as QHints determine the event might make the pod schedulable and then move this pod to backoffQ, this Pod should be able to get retried at any timing, regardless of with or without backoff time.
In case this tons of retries don't make the pod schedulable after all, and make the pod's backoff time much longer, that's the fault of QHint accuracy, not this feature.

The only concern is the backoff time for pods getting error (not unschedulable status) because their scheduling failure isn't solved by events, but solved by some other factors (e.g., the networking issue etc).
In this case, it could happen: pod gets error status -> enqueued directly to backoffQ -> the scheduler pops this pod from backoffQ directly immediately (this feature) -> ... -> the attempt counter is incremented every time and the backoff time gets too long.
But, as the KEP mentioned, we'll address this point along with kubernetes/kubernetes#128748

k8s-ci-robot · 2025-02-11T15:16:38Z

@macsko: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-enhancements-test	`259cf07`	link	true	`/test pull-enhancements-test`
pull-enhancements-verify	`259cf07`	link	true	`/test pull-enhancements-verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

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

k8s-ci-robot requested review from Huang-Wei and kikisdeliveryservice February 6, 2025 16:29

k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 6, 2025

macsko mentioned this pull request Feb 6, 2025

Pop pod from backoffQ when activeQ is empty #5142

Open

4 tasks

macsko force-pushed the pop-Pod_from_backoffq_when-activeq_is_empty branch from 7fa0d54 to 34fc985 Compare February 6, 2025 16:32

k8s-ci-robot requested review from alculquicondor, dom4ha and sanposhiho February 6, 2025 16:35

KEP-5142: Pop pod from backoffQ when activeQ is empty

58bc648

macsko force-pushed the pop-Pod_from_backoffq_when-activeq_is_empty branch from 34fc985 to 58bc648 Compare February 6, 2025 16:38

sanposhiho reviewed Feb 6, 2025

View reviewed changes

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md Outdated Show resolved Hide resolved

keps/sig-scheduling/5142-pop-backoffq-when-activeq-empty/README.md Show resolved Hide resolved

sanposhiho reviewed Feb 7, 2025

View reviewed changes

keps/prod-readiness/sig-scheduling/5142.yaml

@@ -0,0 +1,3 @@

kep-number: 5142

alpha:

approver: "@wojtek-t"

Copy link

Member

sanposhiho Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @wojtek-t

k8s-ci-robot requested a review from wojtek-t February 7, 2025 00:07

Fill all todos and elaborate on some points

d2fc2db

sanposhiho reviewed Feb 8, 2025

View reviewed changes

dom4ha mentioned this pull request Feb 10, 2025

KEP-5007: DRA Device Attach Before Pod Scheduled #5012

Open

macsko added 3 commits February 10, 2025 11:10

Apply comments

e7ebadc

Fix event name

98c5024

Update toc

a205b3e

sanposhiho reviewed Feb 10, 2025

View reviewed changes

dom4ha reviewed Feb 11, 2025

View reviewed changes

macsko added 2 commits February 11, 2025 15:09

Improve wording, change target to beta

afc3295

Update toc

259cf07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-5142: Pop pod from backoffQ when activeQ is empty #5144

KEP-5142: Pop pod from backoffQ when activeQ is empty #5144

macsko commented Feb 6, 2025

k8s-ci-robot commented Feb 6, 2025

macsko commented Feb 6, 2025

sanposhiho left a comment

sanposhiho Feb 7, 2025

sanposhiho left a comment

sanposhiho Feb 10, 2025

macsko Feb 11, 2025

k8s-ci-robot commented Feb 10, 2025

sanposhiho commented Feb 10, 2025

dom4ha Feb 11, 2025

macsko Feb 11, 2025

dom4ha Feb 11, 2025

dom4ha Feb 11, 2025

sanposhiho Feb 11, 2025 •

edited

Loading

sanposhiho Feb 11, 2025 •

edited

Loading

k8s-ci-robot commented Feb 11, 2025

KEP-5142: Pop pod from backoffQ when activeQ is empty #5144

Are you sure you want to change the base?

KEP-5142: Pop pod from backoffQ when activeQ is empty #5144

Conversation

macsko commented Feb 6, 2025

k8s-ci-robot commented Feb 6, 2025

macsko commented Feb 6, 2025

sanposhiho left a comment

Choose a reason for hiding this comment

sanposhiho Feb 7, 2025

Choose a reason for hiding this comment

sanposhiho left a comment

Choose a reason for hiding this comment

sanposhiho Feb 10, 2025

Choose a reason for hiding this comment

macsko Feb 11, 2025

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 10, 2025

sanposhiho commented Feb 10, 2025

dom4ha Feb 11, 2025

Choose a reason for hiding this comment

macsko Feb 11, 2025

Choose a reason for hiding this comment

dom4ha Feb 11, 2025

Choose a reason for hiding this comment

dom4ha Feb 11, 2025

Choose a reason for hiding this comment

sanposhiho Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

sanposhiho Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 11, 2025

sanposhiho Feb 11, 2025 •

edited

Loading

sanposhiho Feb 11, 2025 •

edited

Loading