search

kubernetes-sigs / kube-scheduler-wasm-extension

All the things to make the scheduler extendable with wasm.
Apache License 2.0
86 stars 17 forks source link

Changes guest index back to UID #42

Closed codefromthecrypt closed 11 months ago

codefromthecrypt commented 11 months ago

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

In #39 we switched from using the pod UID to the pointer due to a problem identifying of how to handle the same pod being rescheduled sequentially on error. @sanposhiho found that since PreFilter must always be called, we can reset state always on that hook. A later change can instruct the guest to always dump any cache on PreFilter even if the UID is the same as last time. Meanwhile, this change mechanically switches the host-side index back to the pod UID.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

What are the benchmark results of this change?

goos: darwin
goarch: arm64
pkg: sigs.k8s.io/kube-scheduler-wasm-extension/internal/e2e
                                               │  before.txt  │             after.txt             │
                                               │    sec/op    │   sec/op     vs base              │
PluginFilter/noop-wat/params:_small-12            266.8n ± 0%   263.2n ± 0%  -1.31% (p=0.002 n=6)
PluginFilter/noop-wat/params:_real-12             270.8n ± 0%   268.4n ± 1%  -0.85% (p=0.009 n=6)
PluginFilter/noop/params:_small-12                331.2n ± 1%   327.9n ± 2%       ~ (p=0.084 n=6)
PluginFilter/noop/params:_real-12                 337.2n ± 0%   332.9n ± 2%       ~ (p=0.065 n=6)
PluginFilter/test/params:_small-12                6.486µ ± 2%   6.290µ ± 1%  -3.02% (p=0.002 n=6)
PluginFilter/test/params:_real-12                 115.0µ ± 2%   115.3µ ± 1%       ~ (p=0.589 n=6)
PluginScore/noop-wat/params:_small-12             260.3n ± 0%   257.2n ± 0%  -1.21% (p=0.002 n=6)
PluginScore/noop-wat/params:_real-12              265.5n ± 0%   260.8n ± 1%  -1.75% (p=0.002 n=6)
PluginScore/noop/params:_small-12                 379.5n ± 0%   375.7n ± 1%  -1.00% (p=0.004 n=6)
PluginScore/noop/params:_real-12                  351.8n ± 0%   347.6n ± 3%       ~ (p=0.394 n=6)
PluginScore/test/params:_small-12                 3.619µ ± 1%   3.602µ ± 1%       ~ (p=0.061 n=6)
PluginScore/test/params:_real-12                  40.51µ ± 1%   40.00µ ± 1%       ~ (p=0.093 n=6)
PluginFilterAndScore/noop-wat/params:_small-12    379.2n ± 0%   378.4n ± 1%       ~ (p=0.071 n=6)
PluginFilterAndScore/noop-wat/params:_real-12     384.2n ± 0%   383.3n ± 2%       ~ (p=0.968 n=6)
PluginFilterAndScore/noop/params:_small-12        574.8n ± 0%   572.9n ± 1%       ~ (p=0.145 n=6)
PluginFilterAndScore/noop/params:_real-12         543.1n ± 0%   541.4n ± 0%  -0.31% (p=0.024 n=6)
PluginFilterAndScore/test/params:_small-12       10.091µ ± 1%   9.929µ ± 1%  -1.62% (p=0.004 n=6)
PluginFilterAndScore/test/params:_real-12         168.4µ ± 1%   165.9µ ± 1%  -1.49% (p=0.002 n=6)
geomean                                           1.430µ        1.416µ       -1.02%
sanposhiho commented 11 months ago

/hold

sanposhiho commented 11 months ago

@codefromthecrypt can you check the CI fail? I'm not sure how go.mod is related to this PR's change though.

codefromthecrypt commented 11 months ago

go.mod was related. I should have run make check first.

k8s-ci-robot commented 11 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: codefromthecrypt, 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: - ~~[OWNERS](https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/blob/main/OWNERS)~~ [sanposhiho] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment