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

Expanding on autoscaling templating #101

Closed

Conversation

praktiskt
Copy link
Member

@praktiskt praktiskt commented Oct 10, 2023

Adds:

  • Templated apiVersion for templates/autoscaling.yaml - you can now set autoscaling/v2 if your cluster supports it.
    • Nit: Can we set this as the default? Some comments on the matter here in the original PR suggesting otherwise, but it's been a while since that comment was made...
  • server.autoscaling.extraSpec for templates/autoscaling.yaml - pass whatever you want to spec, meaning you can configure advanced behavior and metrics.

Rationale: While the current implementation of server.autoscaling works for basic cases, it's often desired to expand and customize the behavior of auto scaling further. Some snags I've run into primarily revolves around workers scaling down too fast, causing tasks to fail. Allowing for templating the entire a good chunk of the spec means you can configure things like behavior (slow down up-/downscaling, stabilizationWindowSeconds) and more metrics (e.g. scale on memory limits).

Actual autoscaling behavior is unchanged from previous version to this. Happy to bump this if we agree that autoscaling/v2 is the default (meaning we can also set more sensible defaults for extraSpec).

@cla-bot
Copy link

cla-bot bot commented Oct 10, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@praktiskt
Copy link
Member Author

Signed and sent the CLA.

README.md Outdated Show resolved Hide resolved
@cla-bot
Copy link

cla-bot bot commented Oct 11, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@praktiskt
Copy link
Member Author

CLA accepted by Trino.

@martint
Copy link
Member

martint commented Oct 19, 2023

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Oct 19, 2023
@cla-bot
Copy link

cla-bot bot commented Oct 19, 2023

The cla-bot has been summoned, and re-checked this pull request!

@praktiskt
Copy link
Member Author

@mosabua @martint is there anything more needed before this can be merged?

@praktiskt praktiskt requested a review from mosabua December 8, 2023 18:07
@praktiskt
Copy link
Member Author

Since #120 was merged after I created this (and this PR seems to have been abandoned by Trino maintainers), I'm going to go ahead and close it.

@praktiskt praktiskt closed this Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants