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

ignore sigterm #14

Merged
merged 4 commits into from
Aug 22, 2019
Merged

ignore sigterm #14

merged 4 commits into from
Aug 22, 2019

Conversation

tcrayford
Copy link
Contributor

This sets the l2met-shuttle program to ignore SIGTERM entirely.

This is because, on heroku dynos, the dyno manager sends SIGTERM to every process when doing a graceful shutdown. During that time period, we still want logging to happen and metrics to be sent.

The prior implementation of l2met-shuttle leads to all logs going missing the moment a dyno starts shutting down, and usually that will crash the "main" process l2met-shuttle is a sidecar for.

@tcrayford
Copy link
Contributor Author

It looks like CI has bitrotted :(

tcrayford added a commit to heroku/heroku-buildpack-l2met-shuttle that referenced this pull request Aug 19, 2019
This rewrites `start-l2met-shuttle`, and uses a binary from heroku/l2met-shuttle#14 that ignores `SIGTERM`

This is because, on heroku dynos, the dyno manager sends SIGTERM to every process when doing a graceful shutdown. During that time period, we still want logging to happen and metrics to be sent.

With the prior implementation, the `tee` processes would exit the moment the dyno got told to shutdown.

In the new implementation, `l2met-shuttle` hangs around until the process it's reading from dies.
@tcrayford tcrayford force-pushed the only_sigterm_after_shutting_down branch from dc1a992 to 6eb6687 Compare August 20, 2019 10:48
@edmorley
Copy link
Member

@tt Is the runtime-kill-parent project still alive? This Heroku SIGTERM behaviour causes lots of subtle issues like this and heroku/heroku-buildpack-python#817.

@dzuelke
Copy link

dzuelke commented Aug 20, 2019

Let's pick that work up sometime @edmorley ;)

@tcrayford
Copy link
Contributor Author

For folks reading this later, there's a slack discussion on this here: https://heroku.slack.com/archives/CA5BNBLDR/p1566300448040700

@tcrayford tcrayford merged commit 3ed65a4 into master Aug 22, 2019
@tcrayford tcrayford deleted the only_sigterm_after_shutting_down branch August 22, 2019 10:42
tcrayford added a commit to tcrayford/heroku-buildpack-l2met-shuttle that referenced this pull request Aug 23, 2019
This rewrites `start-l2met-shuttle`, and uses a binary from heroku/l2met-shuttle#14 that ignores `SIGTERM`

This is because, on heroku dynos, the dyno manager sends SIGTERM to every process when doing a graceful shutdown. During that time period, we still want logging to happen and metrics to be sent.

With the prior implementation, the `tee` processes would exit the moment the dyno got told to shutdown.

In the new implementation, `l2met-shuttle` hangs around until the process it's reading from dies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants