fix: repear scheduler_perf to run correctly #116
Conversation
k8s-ci-robot
added
kind/bug
k8s-ci-robot
added
the
size/L
sanposhiho
force-pushed
the
scheduler-perf-run
branch
3 times, most recently
from
7c7b010
to
f90917a
Compare
|
/cc @Gekko0114 @utam0k |
sanposhiho
force-pushed
the
scheduler-perf-run
branch
from
f90917a
to
99df91c
Compare
| @@ -220,6 +220,11 @@ func (g *guest) normalizeScore(ctx context.Context) (framework.NodeScoreList, *f | |||
| statusCode := int32(callStack[0]) | |||
| statusReason := paramsFromContext(ctx).resultStatusReason | |||
| normalizedScoreList := paramsFromContext(ctx).resultNormalizedScoreList | |||
| if len(normalizedScoreList) == 0 { | |||
There was a problem hiding this comment.
nit: What if the length of normalizedScoreList is not equal to the length of nodeScoreList ?
There was a problem hiding this comment.
We return the error:
https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/blob/main/scheduler/plugin/plugin.go#L341-L343
We shouldn't restore the score list in that case because if the length is unmatched, that's the bug in the guest. (The guest filled in an invalid node score list)
We should error out, rather than hiding the bug.
And, what we're trying to do here is not hide the bug. When the guest doesn't implement the normalizescore, we just return without doing anything, which results in an empty resultNormalizedScoreList, and hence fail.
There was a problem hiding this comment.
what we're trying to do here is not hide the bug. When the guest doesn't implement the normalizescore, we just return without doing anything, which results in an empty resultNormalizedScoreList, and hence fail.
Make sense. Thank you so much for your explanation!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Gekko0114, sanposhiho 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:
Approvers can indicate their approval by writing |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
What type of PR is this?
/kind bug
What this PR does / why we need it:
1. Fix a bug with NormalizedScore
Currently, when the guest doesn't implement NormalizedScore, the wasm plugin (scheduler side) errors out, which makes scheduler_perf fail right now.
https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/blob/main/scheduler/plugin/plugin.go#L341-L343
We should restore the node list when the guest returns nil.
2. Create a wasm for scheduler_perf
We're using examples/nodenumber in scheduler_perf, but examples/nodenumber uses many additional features (klog and event recorder) which creates additional overhead.
With scheduler_perf, we just want to compare the performance difference between (default scheduler,) wasm plugin and extender under the same condition (running Score() or
/prioritieswhich only has almost the same logic inside).This PR creates the simplest nodenumber wasm plugin to be used in scheduler_perf for this purpose.
We can add more wasm plugins per test case later. (e.g., the one that uses logging a lot, etc)
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
What are the benchmark results of this change?