-
Notifications
You must be signed in to change notification settings - Fork 518
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.
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
Avoid instantiating separate pod store for every object managed by clusterloader attempt #3 #2103
Conversation
clusterloader2/pkg/measurement/common/wait_for_controlled_pods.go
Outdated
Show resolved
Hide resolved
2d65767
to
8e0bc3f
Compare
/hold cancel I updated the PR description with 5k node test results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach - just added two more technical comments.
…usterloader Co-authored-by: Wojciech Tyczynski <wojtekt@google.com>
PTAL |
/lgtm /hold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mborsz, wojtek-t 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 |
/hold cancel The test results in 5k are already in first comment. |
Based on #2056 and #2016
Compared to #2056 it creates a single informer for all WaitForControlledPodsRunning instances (we have 4 such instances in load test) which cuts memory 4x, which makes memory consumption close to what we had before this PR (approx ~10% growth).
To improve cpu usage it performs indexing by ownerReference and uses it as a hint to find pods related to a given controller.
Comparison of resource consumption by pod running a 5k node test:
Cpu
With this PR:
![image](https://user-images.githubusercontent.com/2559168/179248577-fa670a0e-4b66-4268-a415-fe3ce8652eb5.png)
Baseline:
![image](https://user-images.githubusercontent.com/2559168/179248669-18d5c162-f0d7-4de3-9519-bb6b123069cc.png)
Memory
So CPU usage is comparable to baseline and memory consumption grows from 10GiB to 12 GiB (16Gi is used as a limit for 5k tests).
/kind feature
/assign @wojtek-t