Unicorn graceful shutdown seems incompatible with `TERM` usage
jpcamara opened this issue · 4 comments
First off - glad to be a user of Rack::Timeout! Def a very helpful layer to have.
I am experimenting with the term_on_timeout option with a Unicorn server, and it feels like there is a mismatch between how it functions and what Unicorn expects for a graceful shutdown (which is probably part of the point - a timeout isn't really a graceful shutdown. But utilizing it seems like it would interfere with a general graceful shutdown on restart, for instance).
My unicorn config is partially based on the Heroku documentation for Unicorn: https://devcenter.heroku.com/articles/rails-unicorn#signal-handling
before_fork do |server, worker|
Signal.trap 'TERM' do
puts 'Unicorn master intercepting TERM and sending myself QUIT instead'
Process.kill 'QUIT', Process.pid
end
...
after_fork do |server, worker|
Signal.trap 'TERM' do
puts 'Unicorn worker intercepting TERM and doing nothing. Wait for master to send QUIT'
end
The after_fork handling intercepts and ignores a TERM, which they document the reasoning for:
Unicorn uses the QUIT signal to indicate graceful shutdown. When the master process receives this
signal it sends a QUIT signal to all workers who will then gracefully shut down after completing any in-flight
requests. After the worker processes have shut down, the master process will exit.
Heroku uses the TERM signal to indicate to all processes in a dyno that the dyno is being shut down.
The configuration above ensures that this TERM signal is translated correctly to the Unicorn model:
the workers trap and ignore the signal. The master traps and sends a QUIT signal to itself, thereby
starting the graceful shutdown process.
The only way I can get term_on_timeout to work with Unicorn is by commenting out the Signal.trap 'TERM' in the after_fork. But it seems like that will break the ability to have a graceful shutdown in non-timeout situations.
I didn't see anything Unicorn specific in the PR for this feature, or in the documentation, about handling QUIT vs TERM. Do you have a recommendation on how to handle this?
Thank you!
Isn't the built-in timeout in Unicorn enough? (The Heroku article you linked talks about it)
Isn't the built-in timeout in Unicorn enough? (The Heroku article you linked talks about it)
Wow, thanks for the feedback. Definitely feels like a bit of an "oh duh" moment 😂
I will give that a try and report back. Curious as to what the benefits of using rack-timeout term on timeout would even be with that being available at the unicorn level?
I don't think there's any when using Unicorn :) doc/risks.md do mention Unicorn at one place.
Other web servers, like Puma, might not have such timeout feature and then there's the need for rack-timeout.