Serve buffered client requests before sending the shutdown notice - #1567
Draft
scottjacobsen wants to merge 1 commit into
Draft
scottjacobsen wants to merge 1 commit into
scottjacobsen wants to merge 1 commit into
Conversation
On graceful shutdown the client loop raced the shutdown token against the client socket. A request whose bytes had already been read could be dropped, and a partially received extended-protocol request was wiped when buffer() re-entered and cleared it, so the client saw a backend error or a connection reset instead of the 57P01 notice. The select! is now biased with the shutdown wake-up polled last, the notice is deferred while the stream buffer holds unconsumed bytes or the client request is only partially received, and buffer() only clears a completed request. The shutdown summary reports how many connected clients are idle and notified immediately versus waited on, and each notice is logged at debug level. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Collaborator
|
👁️ |
levkk
reviewed
Sep 17, 2026
| let clients = comms.clients(); | ||
| let idle_clients = clients | ||
| .values() | ||
| .filter(|client| client.stats.state == State::Idle && !client.stats.locked) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
On graceful shutdown (SIGINT or admin
SHUTDOWN)Client::runraced the shutdown token against the client socket:select!order, a request whose bytes had already been read could lose to the shutdown branch and be dropped, and the socket was closed with unread data, so drivers saw a connection reset instead of the 57P01 notice;buffer()poll only to be wiped byclient_request.clear()on re-entry, so the client got a backend error for its Execute+Sync before the FATAL.Changes
select!isbiasedwith the shutdown wake-up polled last, so a request that is ready when the loop polls is served before the notice.stream_bufferholds unconsumed bytes orclient_request.is_partial().buffer()only clears a completed request. This also fixes the same message loss when an async backend message (for example a NotificationResponse) arrived between a client's Parse and Sync.comms().clients()snapshot (idle and not holding a lock).What this does not cover
A request whose bytes arrive after the loop has decided to send the notice is still cut off; that is inherent to a server-initiated close. A client that sends part of a request and then goes quiet is now waited on until
shutdown_timeout, like an idle-in-transaction client, instead of being disconnected immediately.Tests
Two new tests in
graceful_shutdown.rs: a Query already on the socket when shutdown starts is answered before 57P01, and Parse+Bind, shutdown, Execute+Sync completes before 57P01 (fails without thebuffer()change).cargo fmtandcargo clippy --all-targetsare clean; the four graceful shutdown tests pass.Context: found while teaching Rails' ActiveRecord to handle the shutdown notice. libpq treats an ErrorResponse received while idle as a notice, so the next statement hits a closed socket.
🤖 Generated with Claude Code