Store a cluster event to plugin map in SchedulerQueue #98241

Huang-Wei · 2021-01-20T21:35:05Z

What type of PR is this?

/kind feature
/sig scheduling

What this PR does / why we need it:

Part of #94009.

To determine whether a system event can move particular Pods from unschedulableQ/backOff to backoffQ/activeQ, this PR stores a cluster event -> unschedulable plugins in the scheduling queue. On the other hand, if a Pod failed the scheduling with a fitError (in PreFilter, Filter or Permit), we inject the failed plugins to PodInfo. With these 2 info combined together, we're able to move Pods in a fine-grained manner - which I will showcase in the next PR.

Which issue(s) this PR fixes:

The 2nd PR to #94009.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2021-01-20T21:35:12Z

@Huang-Wei: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

Huang-Wei · 2021-01-20T22:50:46Z

/hold
to wait for #98041 to be merged first.

Huang-Wei · 2021-01-21T02:03:22Z

The integration test seems related. I will take a look.

Huang-Wei · 2021-01-21T03:12:04Z

The integration test seems related. I will take a look.

Oops, forgot to add StorageClass into the defaultClusterEvent list.

diff --git a/pkg/scheduler/framework/runtime/framework.go b/pkg/scheduler/framework/runtime/framework.go
index 74e97f33e56..d0500d77f9b 100644
--- a/pkg/scheduler/framework/runtime/framework.go
+++ b/pkg/scheduler/framework/runtime/framework.go
@@ -67,6 +67,7 @@ var defaultClusterEvents = []framework.ClusterEvent{
 	{Resource: framework.PersistentVolume, ActionType: framework.All},
 	{Resource: framework.PersistentVolumeClaim, ActionType: framework.All},
 	{Resource: framework.Service, ActionType: framework.All},
+	{Resource: framework.StorageClass, ActionType: framework.All},
 }
 
 var configDecoder = scheme.Codecs.UniversalDecoder()

Should be good now.

Huang-Wei · 2021-01-21T06:15:34Z

/retest

Huang-Wei · 2021-01-22T03:20:10Z

/retest

ahg-g · 2021-03-02T20:43:08Z

There is clusterEvent, ok := clusterEventReg[event], which is not necessarily trivial. But maybe I'm being too paranoid.

the condition being checked is if len(pInfo.UnschedulablePlugins) != 0 && !p.podMatchesEvent(pInfo, event) {, in the baseline case, just fail the first condition so that podMatchesEvent doesn't even get executed. Basically make sure len(pInfo.UnschedulablePlugins) == 0 for the baseline case.

Huang-Wei · 2021-03-03T00:46:55Z

which is not necessarily trivial. But maybe I'm being too paranoid.

@alculquicondor I ran a test to compare with the previous version. Which doesn't diff a lot.

⇒  benchstat baseline-without-this-PR.txt baseline.txt
name                                  old time/op  new time/op  delta
MoveAllToActiveOrBackoffQueue/1000-8   819µs ± 1%   830µs ± 1%  +1.36%  (p=0.000 n=20+20)
MoveAllToActiveOrBackoffQueue/5000-8  5.21ms ± 2%  5.43ms ± 3%  +4.18%  (p=0.000 n=20+18)

Basically make sure len(pInfo.UnschedulablePlugins) == 0 for the baseline case.

@ahg-g Code updated, and here is the latest testing result:

⇒  benchstat baseline.txt worst.txt
name                                  old time/op  new time/op  delta
MoveAllToActiveOrBackoffQueue/1000-8   830µs ± 1%  1153µs ± 2%  +38.90%  (p=0.000 n=20+19)
MoveAllToActiveOrBackoffQueue/5000-8  5.43ms ± 3%  7.35ms ± 4%  +35.44%  (p=0.000 n=18+20)

⇒  benchstat baseline.txt random.txt
name                                  old time/op  new time/op  delta
MoveAllToActiveOrBackoffQueue/1000-8   829µs ± 1%   494µs ± 2%  -40.42%  (p=0.000 n=18+19)
MoveAllToActiveOrBackoffQueue/5000-8  5.48ms ± 2%  3.28ms ± 2%  -40.20%  (p=0.000 n=19+19)

alculquicondor

It looks like, in the random case, skipping pod moves between queues heavily outweighs the event matching. LGTM

alculquicondor · 2021-03-03T14:21:01Z

pkg/scheduler/framework/runtime/framework_test.go

+		t.Run(tt.name, func(t *testing.T) {
+			got := make(map[framework.ClusterEvent]sets.String)
+			for _, p := range tt.plugins {
+				fillEventToPluginMap(p, got)


Test above looks good, we can remove this one.

Huang-Wei · 2021-03-03T15:22:09Z

It looks like, in the random case, skipping pod moves between queues heavily outweighs the event matching. LGTM

Yes, and once the churn operator PR and the other 2 bug fixes get merged into integration perf test, I will build some integration benchmark tests, so that to get more practical performance results.

Test above looks good, we can remove this one.

Done. Commits squashed.

alculquicondor · 2021-03-03T15:41:12Z

LGTM, but leaving official one to @ahg-g

And it looks like you have to deal with the upgrade to golang 1.16 :(

ahg-g · 2021-03-03T15:51:13Z

looks good, but there are still two commits in this PR.

Huang-Wei · 2021-03-03T18:26:24Z

looks good, but there are still two commits in this PR.

Yes, it's intentional to leave the benchmark test as a separate commit.

And it looks like you have to deal with the upgrade to golang 1.16 :(

Hmm... Let me figure out the CI failure.

alculquicondor · 2021-03-03T18:32:13Z

It's a known failure https://kubernetes.slack.com/archives/C2C40FMNF/p1614780753136400

Huang-Wei · 2021-03-03T20:00:57Z

/retest

ahg-g · 2021-03-03T23:08:35Z

/lgtm
/approve

k8s-ci-robot · 2021-03-03T23:09:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, Huang-Wei

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:

~~pkg/scheduler/OWNERS~~ [Huang-Wei,ahg-g]

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

Huang-Wei · 2021-03-03T23:21:00Z

/hold cancel

dims · 2021-03-04T02:07:35Z

/hold

dims · 2021-03-04T02:11:32Z

/hold cancel
/retest

Huang-Wei · 2021-03-04T03:08:25Z

/retest

fejta-bot · 2021-03-04T10:04:29Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 20, 2021

k8s-ci-robot requested review from damemi and liu-cong January 20, 2021 21:36

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

Huang-Wei force-pushed the sched-enqueue-2 branch from 3b5ab16 to f29bac0 Compare January 20, 2021 22:50

Huang-Wei mentioned this pull request Jan 20, 2021

Avoid moving pods out of unschedulable status unconditionally #94009

Closed

Huang-Wei force-pushed the sched-enqueue-2 branch from f29bac0 to 610deac Compare January 21, 2021 00:52

Huang-Wei mentioned this pull request Jan 21, 2021

PoC: ServiceAffinity enqueue #98248

Closed

Huang-Wei force-pushed the sched-enqueue-2 branch 2 times, most recently from dea9959 to 31ecbb2 Compare January 21, 2021 05:40

Huang-Wei force-pushed the sched-enqueue-2 branch from 31ecbb2 to d4ce973 Compare January 22, 2021 02:30

alculquicondor reviewed Mar 3, 2021

View reviewed changes

Huang-Wei added 2 commits March 3, 2021 07:20

Store a cluster event to plugin map in SchedulerQueue

f322019

sched: add a benchmark test for MoveAllToActiveOrBackoffQueue

6fdb1a8

Huang-Wei force-pushed the sched-enqueue-2 branch from 7a79a7b to 6fdb1a8 Compare March 3, 2021 15:20

k8s-ci-robot assigned ahg-g Mar 3, 2021

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

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

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

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2021

k8s-ci-robot merged commit 60a4998 into kubernetes:master Mar 4, 2021

k8s-ci-robot added this to the v1.21 milestone Mar 4, 2021

Huang-Wei deleted the sched-enqueue-2 branch March 4, 2021 20:00

Huang-Wei mentioned this pull request Mar 8, 2021

sched: cleanup clusterEventReg from pkg/scheduler/internal/queue/events.go #99966

Closed

Huang-Wei mentioned this pull request Mar 17, 2021

[Umbrella] Fine-grained pods enqueuing in scheduler #100347

Closed

sanposhiho mentioned this pull request Jun 2, 2023

support nodeAnnotationsChanged event to trigger rescheduling #118389

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store a cluster event to plugin map in SchedulerQueue #98241

Store a cluster event to plugin map in SchedulerQueue #98241

Huang-Wei commented Jan 20, 2021 •

edited

Loading

k8s-ci-robot commented Jan 20, 2021

Huang-Wei commented Jan 20, 2021

Huang-Wei commented Jan 21, 2021

Huang-Wei commented Jan 21, 2021

Huang-Wei commented Jan 21, 2021

Huang-Wei commented Jan 22, 2021

ahg-g commented Mar 2, 2021

Huang-Wei commented Mar 3, 2021

alculquicondor left a comment

alculquicondor Mar 3, 2021

Huang-Wei commented Mar 3, 2021

alculquicondor commented Mar 3, 2021

ahg-g commented Mar 3, 2021

Huang-Wei commented Mar 3, 2021

alculquicondor commented Mar 3, 2021 •

edited

Loading

Huang-Wei commented Mar 3, 2021

ahg-g commented Mar 3, 2021

k8s-ci-robot commented Mar 3, 2021

Huang-Wei commented Mar 3, 2021

dims commented Mar 4, 2021

dims commented Mar 4, 2021

Huang-Wei commented Mar 4, 2021

fejta-bot commented Mar 4, 2021

Store a cluster event to plugin map in SchedulerQueue #98241

Store a cluster event to plugin map in SchedulerQueue #98241

Conversation

Huang-Wei commented Jan 20, 2021 • edited Loading

k8s-ci-robot commented Jan 20, 2021

Huang-Wei commented Jan 20, 2021

Huang-Wei commented Jan 21, 2021

Huang-Wei commented Jan 21, 2021

Huang-Wei commented Jan 21, 2021

Huang-Wei commented Jan 22, 2021

ahg-g commented Mar 2, 2021

Huang-Wei commented Mar 3, 2021

alculquicondor left a comment

Choose a reason for hiding this comment

alculquicondor Mar 3, 2021

Choose a reason for hiding this comment

Huang-Wei commented Mar 3, 2021

alculquicondor commented Mar 3, 2021

ahg-g commented Mar 3, 2021

Huang-Wei commented Mar 3, 2021

alculquicondor commented Mar 3, 2021 • edited Loading

Huang-Wei commented Mar 3, 2021

ahg-g commented Mar 3, 2021

k8s-ci-robot commented Mar 3, 2021

Huang-Wei commented Mar 3, 2021

dims commented Mar 4, 2021

dims commented Mar 4, 2021

Huang-Wei commented Mar 4, 2021

fejta-bot commented Mar 4, 2021

Huang-Wei commented Jan 20, 2021 •

edited

Loading

alculquicondor commented Mar 3, 2021 •

edited

Loading