perf(scheduler): avoid duplicate full job solver probe#1749
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Performance Benchmark ResultsComparing PR (
|
|
Total coverage: 50.9% -> 51.0% (delta 0.10%) Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
30c4812 to
46b0b78
Compare
07c3812 to
c14ea56
Compare
46b0b78 to
ca421bc
Compare
c14ea56 to
262ebaf
Compare
ca421bc to
224f622
Compare
262ebaf to
91490d6
Compare
224f622 to
e2b530e
Compare
91490d6 to
b197399
Compare
e2b530e to
62da29a
Compare
b197399 to
d266183
Compare
62da29a to
76ed807
Compare
d266183 to
4d54e8c
Compare
76ed807 to
99d1ea9
Compare
4d54e8c to
03caf61
Compare
99d1ea9 to
ce84465
Compare
03caf61 to
0d03317
Compare
ce84465 to
7e8a682
Compare
0d03317 to
b3a3368
Compare
7e8a682 to
3de597b
Compare
b3a3368 to
5e6c486
Compare
3de597b to
c9de803
Compare
5e6c486 to
6808054
Compare
c9de803 to
463df47
Compare
6808054 to
1469386
Compare
463df47 to
0a6b397
Compare
1469386 to
7d3c6e9
Compare
0a6b397 to
04394ea
Compare
| maxSolvedK, searchResult := s.searchMaxSolvableK( | ||
| ssn, state, pendingJob, tasksToAllocate, jobBudget, availableGenerator, generatorBudget, | ||
| ) | ||
| if maxSolvedK == 0 { |
There was a problem hiding this comment.
Why does it have to be equal to 0 instead of smaller then n-1? Why should we attempt to allocate all 3 pods if we cann't find a solution for 2?
89dc5b0 to
0f3b34c
Compare
918290a to
87e08e0
Compare
0f3b34c to
049404c
Compare
87e08e0 to
db50765
Compare
049404c to
e0d19f9
Compare
bbce802 to
06678b1
Compare
e0d19f9 to
9110fe5
Compare
06678b1 to
d5e6ed3
Compare
9110fe5 to
1c4ce47
Compare
d5e6ed3 to
79e14eb
Compare
Signed-off-by: Erez Freiberger <enoodle@gmail.com>
Signed-off-by: Erez Freiberger <enoodle@gmail.com>
1c4ce47 to
44d9149
Compare
Description
Separates a pre-existing
JobSolverperformance fix from the reclaim generator portfolio stack.The old solver could run the full
probeAtK(n)twice: once duringsearchMaxSolvableKand once again inSolveto rebuild the live statement. This PR keepssearchMaxSolvableKto partial probes (k < n) and leaves the full probe to the solve path, so the full allocation is simulated once.References:
docs/developer/designs/reclaim-generator-portfolio-design.md)Related Issues
Related to #1660.
Checklist
Breaking Changes
None.
Additional Notes
This PR is stacked after #1748 so the reclaim generator portfolio and benchmark PRs can be reviewed without attributing this standalone solver optimization to them.
Verification:
GOCACHE=/tmp/kai-reclaim-generator-go-cache go test ./pkg/scheduler/plugins/scenariogenerators ./pkg/scheduler/conf_util ./pkg/operator/operands/scheduler ./pkg/scheduler/actions/common/solvers ./pkg/scheduler/actions/reclaim ./pkg/scheduler/actions/preempt ./pkg/scheduler/actions/consolidation ./pkg/scheduler/actions/integration_tests/reclaim ./pkg/scheduler/cache ./pkg/scheduler/cache/status_updater ./pkg/scheduler/metrics ./pkg/apis/scheduling/v2alpha2 ./pkg/apis/kai/v1 -count=1git diff --check