-
Notifications
You must be signed in to change notification settings - Fork 0
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
Graceful shutdown #37
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Love all the comments, as usual. Only thing I'm wondering about, is why not use the oneshot channel?
/// Shutdown channel. Used to indicate that we should start graceful shutdown. | ||
/// The channel has capacity 1 as we only need to signal once to shutdown. | ||
/// Missing messages on the channel doesn't matter. | ||
shutdown: broadcast::Sender<()>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use a oneshot channel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried but oneshot is single-producer single-consumer, and we need to both send from multiple places (internally and the user can also) and we need to receive in multiple places (each handler) so the broadcast channel is the only one that satisfies that in tokio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 makes sense
The app will now gracefully shut down when signalled through a broadcast channel. A convenience function is also added that automatically listens for ctrl-c (non-Unix) or SIGTERM, SIGINT and SIGHUP (Unix).