-
Notifications
You must be signed in to change notification settings - Fork 170
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
RUST-1608 Clean shutdown for Client #920
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.
code changes look good, although it looks like there are some lint/test failures in CI
Ah, yep, fixed the lint, and I'd forgotten to restrict the transaction-using tests to transaction-supporting servers. The cargo-deny failure is unrelated (see RUST-1674 for details). |
Updated with changes to unify the handling for |
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! nice work with this
RUST-1608
This adds the
shutdown
andshutdown_immediate
methods onClient
. Terminating the workers was straightforward but closing outstanding resources (cursors and sessions) was very much not. Initially, I was hoping thatshutdown
could actively track and clean up those when called, but the shared ownership required for that would have required API changes that were both backwards-incompatible and generally much less easy to work with.What I've ended up with is using the pre-created tasks we discussed in RUST-1439; while this doesn't mean they're guaranteed to run when the runtime shuts down, it does mean that the
Client
can track and await them. Concretely, this allowsshutdown
to wait for all pending resources to have been dropped and complete async cleanup. This also fixes RUST-1099.This solution does have a couple of downsides:
shutdown
will return. For normal usage, this shouldn't be an issue - Rust's typical scoped memory management should mean that most of the time that happens automatically, and when it doesn't it should be easy to fix. In case that's not possible,shutdown_immediate
provides an escape hatch that does just the worker thread termination without the resource wait.shutdown
has waited for resources but before it closes out the workers. I think this mostly falls into the "please don't do that" sort of misbehavior.