scheduler: fix perf downgrade of cases without presence of (anti-)affinity pods #76973
Conversation
k8s-ci-robot
added
the
release-note-none
k8s-ci-robot
added
kind/bug
Huang-Wei
force-pushed
the
lazy-init-int64-ptr
branch
from
4bc5476
to
492b970
Compare
k8s-ci-robot
added
the
approved
bsalamat
requested review from
bsalamat
and removed request for
smarterclayton and
thockin
There was a problem hiding this comment.
Thanks, @Huang-Wei! It looks good. I hope this one behaves as expected.
I also see a couple other potential optimizations in finding min/max, but let's first see if this PR improves performance. We can add those in follow-up PRs.
| } | ||
| } | ||
|
|
||
| // calculate final priority score for each node | ||
| result := make(schedulerapi.HostPriorityList, 0, len(nodes)) | ||
| maxMinDiff := maxCount - minCount |
There was a problem hiding this comment.
It looks to me that if maxMinDiff is zero, we can skip the for loop below altogether.
There was a problem hiding this comment.
Thanks for the review!
Yes, that could be possible. I can investigate later to see whether current Priority reduce interface is fine with an empty []schedulerapi.HostPriorityList vs. mandatory entries with schedulerapi.HostPriority{Host: node.Name, Score: 0} in the result set.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bsalamat, Huang-Wei 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 |
|
/retest |
What type of PR is this?
/kind bug
/sig scheduling
/assign @bsalamat @ravisantoshgudimetla
What this PR does / why we need it:
Some background: scheduler: performance improvement on PodAffinity #76243 was introduced to improve pod affinity, but it sorts of regress other usecases, so we reverted that PR.
Root cause of old PR: For a typical kubemark test, it just runs some regular workloads. No affinity pods are involved in. With that said, the mutex in inter-podaffinity priority is actually a non-op. So, what's the exact overhead old pr brings in? I did a test and it turns out the allocation of *int64 pointer for each node is the culprit.
How this PR fixes the issue: This PR introduces a dynamic init mechanism for those *int64 pointers. Then in the final score calculation phase, if it's not inited, we simply assign score 0 to it.
Benchmark tests for Inter-PodAffinity Priority: It shows old PR has 60%+ overhead (for cases without affinity pods), and new(this) PR has zero overhead.
Can this PR still help PodAffinity cases: Yes.
Which issue(s) this PR fixes:
A robust version of #76243.
Special notes for your reviewer:
If you have already reviewed the old PR, just take a look at commit 4bc5476.
Does this PR introduce a user-facing change?:
(I guess we shouldn't mention it in release-note again, yes or no?)