Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change EvaluatedNodes to count Nodes that reach Filter phase only #124735

Merged
merged 1 commit into from
May 10, 2024
+25 −8
Merged

Change EvaluatedNodes to count Nodes that reach Filter phase only #124735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/scheduler/framework/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,9 @@ type Diagnosis struct {
PreFilterMsg string
// PostFilterMsg records the messages returned from PostFilter plugins.
PostFilterMsg string
// EvaluatedNodes records the number of nodes evaluated by Filter stage.
// It is used for debugging purposes only.
EvaluatedNodes int
}

// FitError describes a fit error of a pod.
Expand Down
7 changes: 5 additions & 2 deletions pkg/scheduler/schedule_one.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ func (sched *Scheduler) schedulePod(ctx context.Context, fwk framework.Framework
if len(feasibleNodes) == 1 {
return ScheduleResult{
SuggestedHost: feasibleNodes[0].Node().Name,
EvaluatedNodes: 1 + len(diagnosis.NodeToStatusMap),
EvaluatedNodes: diagnosis.EvaluatedNodes,
FeasibleNodes: 1,
}, nil
}
Expand All @@ -432,7 +432,7 @@ func (sched *Scheduler) schedulePod(ctx context.Context, fwk framework.Framework

return ScheduleResult{
SuggestedHost: host,
EvaluatedNodes: len(feasibleNodes) + len(diagnosis.NodeToStatusMap),
EvaluatedNodes: diagnosis.EvaluatedNodes,
FeasibleNodes: len(feasibleNodes),
}, err
}
Expand Down Expand Up @@ -594,6 +594,7 @@ func (sched *Scheduler) findNodesThatPassFilters(
for i := range feasibleNodes {
feasibleNodes[i] = nodes[(sched.nextStartNodeIndex+i)%numAllNodes]
}
diagnosis.EvaluatedNodes = int(numNodesToFind)
return feasibleNodes, nil
}

Expand Down Expand Up @@ -642,11 +643,13 @@ func (sched *Scheduler) findNodesThatPassFilters(
// are found.
fwk.Parallelizer().Until(ctx, numAllNodes, checkNode, metrics.Filter)
feasibleNodes = feasibleNodes[:feasibleNodesLen]
diagnosis.EvaluatedNodes = int(feasibleNodesLen)
for _, item := range result {
if item == nil {
continue
}
diagnosis.NodeToStatusMap[item.node] = item.status
diagnosis.EvaluatedNodes++
diagnosis.AddPluginStatus(item.status)
}
if err := errCh.ReceiveError(); err != nil {
Expand Down
23 changes: 17 additions & 6 deletions pkg/scheduler/schedule_one_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,7 @@ func TestSchedulerNoPhantomPodAfterDelete(t *testing.T) {
node.Name: framework.NewStatus(framework.Unschedulable, nodeports.ErrReason).WithPlugin(nodeports.Name),
},
UnschedulablePlugins: sets.New(nodeports.Name),
EvaluatedNodes: 1,
},
}
if !reflect.DeepEqual(expectErr, err) {
Expand Down Expand Up @@ -1042,6 +1043,7 @@ func TestSchedulerFailedSchedulingReasons(t *testing.T) {
Diagnosis: framework.Diagnosis{
NodeToStatusMap: failedNodeStatues,
UnschedulablePlugins: sets.New(noderesources.Name),
EvaluatedNodes: 100,
},
}
if len(fmt.Sprint(expectErr)) > 150 {
Expand Down Expand Up @@ -1829,6 +1831,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"node2": framework.NewStatus(framework.Unschedulable, tf.ErrReasonFake).WithPlugin("FalseFilter"),
},
UnschedulablePlugins: sets.New("FalseFilter"),
EvaluatedNodes: 2,
},
},
},
Expand Down Expand Up @@ -1919,6 +1922,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"1": framework.NewStatus(framework.Unschedulable, tf.ErrReasonFake).WithPlugin("FalseFilter"),
},
UnschedulablePlugins: sets.New("FalseFilter"),
EvaluatedNodes: 3,
},
},
},
Expand All @@ -1945,6 +1949,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"2": framework.NewStatus(framework.Unschedulable, tf.ErrReasonFake).WithPlugin("NoPodsFilter"),
},
UnschedulablePlugins: sets.New("MatchFilter", "NoPodsFilter"),
EvaluatedNodes: 2,
},
},
},
Expand Down Expand Up @@ -2110,6 +2115,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"3": framework.NewStatus(framework.Unschedulable, "injecting failure for pod test-filter").WithPlugin("FakeFilter"),
},
UnschedulablePlugins: sets.New("FakeFilter"),
EvaluatedNodes: 1,
},
},
},
Expand Down Expand Up @@ -2143,6 +2149,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"3": framework.NewStatus(framework.Unschedulable, "injecting failure for pod test-filter").WithPlugin("FakeFilter"),
},
UnschedulablePlugins: sets.New("FakeFilter", framework.ExtenderName),
EvaluatedNodes: 3,
Copy link
Member

Choose a reason for hiding this comment

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

why are these values changing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I added a new field to type Diagnosis struct.
And in these test cases, we use reflect.deepequal to compare two fitError, so it will compare the new field EvaluatedNodes too.
It is set to 0 default, which is inconsistent with the real situation.

},
},
},
Expand All @@ -2168,6 +2175,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"3": framework.NewStatus(framework.UnschedulableAndUnresolvable, "injecting failure for pod test-filter").WithPlugin("FakeFilter"),
},
UnschedulablePlugins: sets.New("FakeFilter"),
EvaluatedNodes: 1,
},
},
},
Expand Down Expand Up @@ -2249,7 +2257,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
nodes: []string{"node1", "node2", "node3"},
pod: st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
wantNodes: sets.New("node2"),
wantEvaluatedNodes: ptr.To[int32](3),
wantEvaluatedNodes: ptr.To[int32](1),
},
{
name: "test prefilter plugin returning non-intersecting nodes",
Expand Down Expand Up @@ -2338,6 +2346,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
"node2": framework.NewStatus(framework.Unschedulable, "injecting failure for pod test-prefilter").WithPlugin("FakeFilter"),
},
UnschedulablePlugins: sets.New("FakeFilter"),
EvaluatedNodes: 1,
PreFilterMsg: "",
},
},
Expand Down Expand Up @@ -2416,10 +2425,11 @@ func TestSchedulerSchedulePod(t *testing.T) {
),
tf.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
},
nodes: []string{"node1", "node2", "node3"},
pod: st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
wantNodes: sets.New("node1", "node2"),
wantEvaluatedNodes: ptr.To[int32](2),
nodes: []string{"node1", "node2", "node3"},
pod: st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
wantNodes: sets.New("node1", "node2"),
// since this case has no score plugin, we'll only try to find one node in Filter stage
wantEvaluatedNodes: ptr.To[int32](1),
},
}
for _, test := range tests {
Expand Down Expand Up @@ -2483,7 +2493,7 @@ func TestSchedulerSchedulePod(t *testing.T) {
if gotOK != wantOK {
t.Errorf("Expected err to be FitError: %v, but got %v (error: %v)", wantOK, gotOK, err)
} else if gotOK {
if diff := cmp.Diff(gotFitErr, wantFitErr); diff != "" {
if diff := cmp.Diff(wantFitErr, gotFitErr); diff != "" {
t.Errorf("Unexpected fitErr: (-want, +got): %s", diff)
}
}
Expand Down Expand Up @@ -2536,6 +2546,7 @@ func TestFindFitAllError(t *testing.T) {
"3": framework.NewStatus(framework.Unschedulable, tf.ErrReasonFake).WithPlugin("MatchFilter"),
},
UnschedulablePlugins: sets.New("MatchFilter"),
EvaluatedNodes: 3,
}
if diff := cmp.Diff(diagnosis, expected); diff != "" {
t.Errorf("Unexpected diagnosis: (-want, +got): %s", diff)
Expand Down