fix: skip isPodWorthRequeuing only when SchedulingGates gates the pod

This commit is contained in:
Kensei Nakada 2024-06-15 22:36:42 +00:00
parent 85fad5b874
commit a805b38229
3 changed files with 33 additions and 11 deletions

View File

@ -47,6 +47,7 @@ import (
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/scheduler/framework"
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity"
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
"k8s.io/kubernetes/pkg/scheduler/internal/heap"
"k8s.io/kubernetes/pkg/scheduler/metrics"
"k8s.io/kubernetes/pkg/scheduler/util"
@ -1175,11 +1176,25 @@ func (p *PriorityQueue) movePodsToActiveOrBackoffQueue(logger klog.Logger, podIn
activated := false
for _, pInfo := range podInfoList {
// Since there may be many gated pods and they will not move from the
// unschedulable pool, we skip calling the expensive isPodWorthRequeueing.
if pInfo.Gated {
// When handling events takes time, a scheduling throughput gets impacted negatively
// because of a shared lock within PriorityQueue, which Pop() also requires.
//
// Scheduling-gated Pods never get schedulable with any events,
// except the Pods themselves got updated, which isn't handled by movePodsToActiveOrBackoffQueue.
// So, we can skip them early here so that they don't go through isPodWorthRequeuing,
// which isn't fast enough to keep a sufficient scheduling throughput
// when the number of scheduling-gated Pods in unschedulablePods is large.
// https://github.com/kubernetes/kubernetes/issues/124384
// This is a hotfix for this issue, which might be changed
// once we have a better general solution for the shared lock issue.
//
// Note that we cannot skip all pInfo.Gated Pods here
// because PreEnqueue plugins apart from the scheduling gate plugin may change the gating status
// with these events.
if pInfo.Gated && pInfo.UnschedulablePlugins.Has(names.SchedulingGates) {
continue
}
schedulingHint := p.isPodWorthRequeuing(logger, pInfo, event, oldObj, newObj)
if schedulingHint == queueSkip {
// QueueingHintFn determined that this Pod isn't worth putting to activeQ or backoffQ by this event.

View File

@ -43,6 +43,7 @@ import (
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/scheduler/framework"
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort"
"k8s.io/kubernetes/pkg/scheduler/metrics"
st "k8s.io/kubernetes/pkg/scheduler/testing"
@ -1447,13 +1448,19 @@ func TestPriorityQueue_MoveAllToActiveOrBackoffQueueWithQueueingHint(t *testing.
expectedQ: unschedulablePods,
},
{
name: "QueueHintFunction is not called when Pod is gated",
podInfo: setQueuedPodInfoGated(&framework.QueuedPodInfo{PodInfo: mustNewPodInfo(p), UnschedulablePlugins: sets.New("foo")}),
name: "QueueHintFunction is not called when Pod is gated by SchedulingGates plugin",
podInfo: setQueuedPodInfoGated(&framework.QueuedPodInfo{PodInfo: mustNewPodInfo(p), UnschedulablePlugins: sets.New(names.SchedulingGates, "foo")}),
hint: func(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
return framework.Queue, fmt.Errorf("QueueingHintFn should not be called as pod is gated")
},
expectedQ: unschedulablePods,
},
{
name: "QueueHintFunction is called when Pod is gated by a plugin other than SchedulingGates",
podInfo: setQueuedPodInfoGated(&framework.QueuedPodInfo{PodInfo: mustNewPodInfo(p), UnschedulablePlugins: sets.New("foo")}),
hint: queueHintReturnQueue,
expectedQ: activeQ,
},
}
for _, test := range tests {
@ -1466,14 +1473,13 @@ func TestPriorityQueue_MoveAllToActiveOrBackoffQueueWithQueueingHint(t *testing.
QueueingHintFn: test.hint,
},
}
test.podInfo.UnschedulablePlugins = sets.New("foo")
cl := testingclock.NewFakeClock(now)
q := NewTestQueue(ctx, newDefaultQueueSort(), WithQueueingHintMapPerProfile(m), WithClock(cl))
// add to unsched pod pool
q.activeQ.Add(q.newQueuedPodInfo(test.podInfo.Pod))
if p, err := q.Pop(logger); err != nil || p.Pod != test.podInfo.Pod {
t.Errorf("Expected: %v after Pop, but got: %v", test.podInfo.Pod.Name, p.Pod.Name)
}
// add to unsched pod pool
err := q.AddUnschedulableIfNotPresent(logger, test.podInfo, q.SchedulingCycle())
if err != nil {
t.Fatalf("unexpected error from AddUnschedulableIfNotPresent: %v", err)

View File

@ -2629,8 +2629,8 @@ func (pl *SchedulingGatesPluginWOEvents) EventsToRegister() []framework.ClusterE
return nil
}
// This test helps to verify registering nil events for schedulingGates plugin works as expected.
func TestSchedulingGatesPluginEventsToRegister(t *testing.T) {
// This test helps to verify registering nil events for PreEnqueue plugin works as expected.
func TestPreEnqueuePluginEventsToRegister(t *testing.T) {
testContext := testutils.InitTestAPIServer(t, "preenqueue-plugin", nil)
num := func(pl framework.Plugin) int {
@ -2648,7 +2648,8 @@ func TestSchedulingGatesPluginEventsToRegister(t *testing.T) {
tests := []struct {
name string
enqueuePlugin framework.PreEnqueuePlugin
count int
// count is the expected number of calls to PreEnqueue().
count int
}{
{
name: "preEnqueue plugin without event registered",
@ -2658,7 +2659,7 @@ func TestSchedulingGatesPluginEventsToRegister(t *testing.T) {
{
name: "preEnqueue plugin with event registered",
enqueuePlugin: &SchedulingGatesPluginWithEvents{SchedulingGates: schedulinggates.SchedulingGates{}},
count: 2,
count: 3,
},
}