-
Notifications
You must be signed in to change notification settings - Fork 734
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
[Autoscaling] Add CPU recommender #5924
Conversation
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.
Nice work! 👍
In my testing, I think at least, I was not able to observe CPU based autoscaling but that might be due to the simulated load I was using that required more storage more than anything else.
One thing I noticed is that the ML autoscaling I had in place went a bit crazy and requested 9 ML nodes with 8 CPU each only to scale them down later (without me having created any ML models) I could not find a flaw in you implementation though and think this is purely due to the decision Elasticsearch made.
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 didn't test in real but code LGTM.
Left some nitpicking on wording.
pkg/controller/autoscaling/elasticsearch/autoscaler/recommender/cpu.go
Outdated
Show resolved
Hide resolved
pkg/controller/autoscaling/elasticsearch/autoscaler/recommender/cpu.go
Outdated
Show resolved
Hide resolved
pkg/controller/autoscaling/elasticsearch/autoscaler/recommender/cpu.go
Outdated
Show resolved
Hide resolved
pkg/controller/autoscaling/elasticsearch/autoscaler/recommender/cpu.go
Outdated
Show resolved
Hide resolved
pkg/controller/autoscaling/elasticsearch/autoscaler/recommender/cpu.go
Outdated
Show resolved
Hide resolved
...roller/autoscaling/elasticsearch/testdata/cpu-scaled-horizontally/elasticsearch-expected.yml
Show resolved
Hide resolved
This commit adds a CPU recommender to update the required Pods CPU according to the "processors" field in the Elasticsearch autoscaling capacity response.
This PR adds a CPU recommender to the Autoscaling controller.
Fixes #5823