-
Notifications
You must be signed in to change notification settings - Fork 241
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
base: main
Are you sure you want to change the base?
Conversation
b58b484
to
92e1548
Compare
Also: - use util::task::spawn_blocking in place of tokio::task::spawn_blocking
...if shutdown in graceful.
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
Co-authored-by: sistemd <[email protected]>
Co-authored-by: sistemd <[email protected]>
version = { workspace = true } | ||
authors = { workspace = true } | ||
edition = { workspace = true } | ||
license = { workspace = true } | ||
rust-version = { workspace = true } | ||
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html |
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.
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html |
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.
It's a copy-paste thing, should I remove it from all crates?
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.
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,