-
Notifications
You must be signed in to change notification settings - Fork 246
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
Implement graceful shutdown #2456
Conversation
92e1548
to
f380908
Compare
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.
LGTM
task_tracker.spawn(async move { | ||
tokio::select! { | ||
_ = cancellation_token.cancelled() => { | ||
F::Output::cancelled() |
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.
Here.
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.
Otherwise the signature would have to be
pub fn spawn<F>(future: F) -> tokio::task::JoinHandle<Option<F::Output>>
to return None
if gracefully cancelled.
Also: - use util::task::spawn_blocking in place of tokio::task::spawn_blocking
...if shutdown in graceful.
Co-authored-by: sistemd <[email protected]>
95c98e1
to
423d84a
Compare
Fixes: #2417
tokio::task
-s andtokio
blocking tasks (spawn_blocking
) are now spawned and tracked via a task tracker, using theutil::task::spawn[_blocking]
api,spawn_blocking
, and the caller must use it to bail out in case of a longer running task,std::thread
-s should now be spawned withutil::task::spawn_std
, because it gives the caller access to the token too and the caller should then use it to break out of a long running loop,util::task::spawn[_blocking]
tasks will be waited upon, whileutil::task::spawn_std
will not be joined, unless the caller locally does so (we don't use many and I don't think their detachment is a big issue),INT
andTERM
signal listeners are installed after the db is migrated and tries are pruned to allow the user to cancel that process instead of forcing them to wait,Apart from implementing the above ⬆️ , adds a refactor plus some fixes to bugs noticed on the way, as per commit order:
util
crate for general Rust utilities,rayon
wherespawn_blocking
is sub-optimal,make_stream
andAnyhowExt
toutil
,rollback_to_anchor
does notcommit()
the db transaction,