Skip to content
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

Allow autoscaling by memory utilization #239

Merged

Conversation

dantonbertuol
Copy link
Contributor

Hello!

This PR aims to enable the possibility of using memory resource in the HPA. I noticed that there is an open PR (#219) regarding this, but it has been inactive for a month, and this is a feature I really need.

A new variable has been created in the values (targetMemoryUtilizationPercentage), with a default value of 80. In the HPA manifest, I added a conditional check to see if a value is set for this variable, allowing the assignment of an empty value ("") to one of them to not use the metric. I also included this condition for targetCPUUtilizationPercentage (since someone might not want to scale based on CPU).

@cla-bot cla-bot bot added the cla-signed label Oct 3, 2024
Copy link
Member

@nineinchnick nineinchnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for preparing this, just a small nitpick about wording.

@nineinchnick nineinchnick changed the title feat: enable autoscaling by memory resource Allow autoscaling by memory utilization Oct 3, 2024
@nineinchnick
Copy link
Member

Also, please remove the feat: from the commit message. I update the PR title, you can use it as the commit message.

@dantonbertuol dantonbertuol force-pushed the feature/memory-resource-autoscaling branch from 569e066 to 340b393 Compare October 3, 2024 13:43
@dantonbertuol dantonbertuol force-pushed the feature/memory-resource-autoscaling branch from 340b393 to 7f0dd0e Compare October 3, 2024 13:46
@nineinchnick nineinchnick added the enhancement New feature or request label Oct 3, 2024
@nineinchnick nineinchnick merged commit 476af36 into trinodb:main Oct 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants