Fix: cancel backend sessions when shutdown timeout expires #524
Conversation
|
|
pgdog/src/frontend/listener.rs
Outdated
| ); | ||
|
|
||
| // Shutdown timeout elapsed; cancel any still-running queries before tearing pools down. | ||
| let cancel_futures = comms.clients().into_keys().map(|id| async move { |
There was a problem hiding this comment.
This is good. One note: let's make this code run conditionally on shutdown_termination_timeout being configured. Otherwise, this is good to go.
There was a problem hiding this comment.
I had read #517 as “cancel queries by default on shutdown, and use shutdown_termination_timeout only for how long we wait.”
If we guard the cancellation logic behind that option, we’d fall back to not cancelling at all.
Was the intent to keep cancellation optional, or should we still do it by default and let the timeout act purely as a safeguard?
There was a problem hiding this comment.
If we guard the cancellation logic behind that option, we’d fall back to not cancelling at all.
That's right, we want this feature to be optional. If we allow query cancellation without a timeout, it can block pgdog from shutting down if the network is unreliable.
The timeout is a safeguard, and we can't use this feature without it safely.
There was a problem hiding this comment.
Thanks for clarifying!
I'll modify the code to only run cancellation when the timeout is set
There was a problem hiding this comment.
Done. The cancellation now only runs when shutdown_termination_timeout is configured (updated in the latest commit).
Heads-up: once this lands, I’ll open an issue in the docs repo and follow up with the config update there.
3 participants
Related Issue
Description
CANCELrequests to Postgres before tearing down connection pools, preventing long-running queries from lingering.Code changes
execute_shutdownwhen shutdown timeout expires.Testing