trinodb / charts

Apache License 2.0
151 stars 174 forks source link

feat: :sparkles: Added Lifecycle Hook and Termination Grace Period #94

Closed danielcrisap closed 6 months ago

danielcrisap commented 1 year ago

Those features are intended to achieve the Graceful shutdown from Trino Worker nodes. https://trino.io/docs/current/admin/graceful-shutdown.html#admin-graceful-shutdown

With lifecycle hooks, when the pods receive a SIGTERM will request the API to start the SHUTTING_DOWN state and the terminationGracePeriodSeconds is for K8s waiting for this process to be concluded.

cla-bot[bot] commented 1 year ago

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 cla@trino.io. 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

tbaeg commented 11 months ago

+1 for this feature.

Would be a nice feature to have on the coordinator as well. I can branch this PR and create another one since I'm unsure when the CLA will be signed.

mosabua commented 11 months ago

@cla-bot check

cla-bot[bot] commented 11 months ago

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 cla@trino.io. 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

cla-bot[bot] commented 11 months ago

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

danielcrisap commented 11 months ago

Sorry for the delay,

I've sent the email with the CLA signed; I will wait a few days to recheck.

tbaeg commented 11 months ago

@danielcrisap Any chance you could also add these to the coordinator?

cla-bot[bot] commented 11 months ago

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 cla@trino.io. 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

danielcrisap commented 11 months ago

@danielcrisap Any chance you could also add these to the coordinator?

I included the coordinator; I just removed the example because, in the Trino doc, this API is only for Worker nodes.

Trino has a graceful shutdown API that can be used exclusively on workers in order to ensure that they terminate without affecting running queries, given a sufficient grace period.

mosabua commented 11 months ago

Well.. there is a graceful shutdown for coordinator in the works...

tbaeg commented 11 months ago

@danielcrisap Any chance you could also add these to the coordinator?

I included the coordinator; I just removed the example because, in the Trino doc, this API is only for Worker nodes.

Trino has a graceful shutdown API that can be used exclusively on workers in order to ensure that they terminate without affecting running queries, given a sufficient grace period.

Awesome, thank you! Yeah, mostly looking for the lifecycle option as it provides flexibility for some custom operations to be executed. Appreciate you including it!

cla-bot[bot] commented 11 months ago

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 cla@trino.io. 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

danielcrisap commented 11 months ago

@cla-bot check

cla-bot[bot] commented 11 months ago

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

avinashdesireddy commented 10 months ago

I'm interested in this feature, Can this be merged? Happy to support any additional changes here.

danielcrisap commented 8 months ago

@tbaeg and @mosabua , Do you think we can include this feature in the next release?

tbaeg commented 8 months ago

No issues from me but I also have no decision making authority. :)

mosabua commented 7 months ago

@radek-starburst @losipiuk @nineinchnick .. any chance you could look at this?

danielcrisap commented 6 months ago

It looks ready to be merged.

Please squash all commits, see https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#git-merge-strategy for more info.

I don't have merge permission, so I can't set to use the squad strategy for merging into main.

nineinchnick commented 6 months ago

You can squash commits locally and force-push to this branch.

danielcrisap commented 6 months ago

You can squash commits locally and force-push to this branch.

Squashed into a single commit ✅