Repository navigation
fix(sql-pg): retire a session whose CancelRequest may be in flight - #8306
Closed
kristojorg wants to merge 1 commit into
Closed
kristojorg wants to merge 1 commit into
kristojorg wants to merge 1 commit into
Conversation
A CancelRequest travels on a side connection and the backend applies it to whatever it is running when it arrives. Nothing on the wire confirms delivery: a pooler or hosted proxy may close the side connection before it forwards the request. An interrupted query or an aborted stream that then completes on its own returns the session to the pool with the cancel still on its way, and the next statement on that session fails with StatementTimeoutError (SQLSTATE 57014). Track whether the backend reported the cycle's statement cancelled. If it did, the session stays usable as before. Otherwise close it with a ConnectionError so the pool replaces it. The new tests put a Duplex between client and backend that acknowledges a CancelRequest at once and forwards it 250 ms later, which is what a pooler does, and check that the next statement on the pool is not cancelled. Both fail on the previous code with 57014.
🦋 Changeset detectedLatest commit: 4f7e4da The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
This branch is waiting to be deployed
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
1 participant
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.
Summary
PgConnection.queryandPgConnection.streamcancel an interrupted statement with aCancelRequest, drain toReadyForQuery, and keep the session. But aCancelRequestis asynchronous by protocol design: it travels on a side connection, the backend applies it to whatever it is running when it arrives, and nothing on the wire confirms it was delivered.A pooler or hosted proxy may close the side connection before it forwards the request. If the interrupted statement finishes on its own in the meantime, the session goes back to the pool looking healthy — and the cancel lands on the next caller instead.
The race
sequenceDiagram participant A as Caller A participant P as Pool participant X as Pooler or proxy participant PG as PostgreSQL participant B as Caller B A->>PG: SELECT ... on session S Note over A: Effect.timeout fires, interrupt A->>X: CancelRequest on a side connection X-->>A: closes the side connection, looks delivered PG-->>A: statement completes on its own Note over A,PG: drain reaches ReadyForQuery, S looks healthy A->>P: return S B->>P: get P-->>B: S B->>PG: SELECT ... on S X->>PG: CancelRequest, forwarded at last PG-->>B: ERROR 57014 canceling statement due to user requestCaller B gets a
SqlErrorwithStatementTimeoutErroron a statement nobody interrupted. We hit this in CI against Neon.The existing test
interrupt cancels an in-flight querycovers only the happy ordering, where the cancel arrives while the statement is still running.The fix
The backend tells us whether the cancel actually landed: it reports
57014for the cancelled statement.QueryMachineand the stream consumer now watch for that, before or during the drain, and decide from it.The multiplexed unpinned path never cancels and is unchanged.
57014is also whatstatement_timeoutraises, so a timeout that coincides with an interrupt is read as the cancel having landed. Separating the two would mean matching the message text, which is locale dependent.What the retirement costs
For pooled callers the change is invisible: the pool discards the session and opens another.
Callers holding a connection from
PgConnection.makedirectly see a behavior change. After an interrupt whose cancel was not confirmed, that connection is dead, and every later use fails withConnectionErrorexactly as it would after a dropped socket. Alistenqueue on the session fails with the same error, andStream.retryreconnects.Alternatives considered
CancelRequestto the postmaster over a new connection precisely because an executing backend does not read the frontend socket.57014and nothing is in flight.client_connection_check_intervalis off by default, so a long statement can run to completion.57014on its next statement. A spurious cancel cannot be told apart from a realstatement_timeoutor a legitimate concurrent cancel, so this would silently retry statements that were cancelled on purpose.Connection churn is bounded by how often statements are interrupted, which the protocol documentation already assumes is infrequent. Only an interrupt that loses the race retires a session; one whose cancel the backend confirmed does not.
Tests
PgPool.integration.test.tsgains aLateCancelProxyDuplex, passed to the pool as the driver'sstreamoption. It is thePooler or proxyparticipant above: it forwards every byte, but acknowledges aCancelRequestat once and delivers it to the backend 250 ms later.Two new tests drive the diagram end to end, one interrupting a query and one aborting a stream, then run a statement on the pool and assert it is not cancelled.
Validation
packages/sql/pgsuite against testcontainers withEFFECT_INTEGRATION_TESTS=1: 403 pass, 15 files, no failures.StatementTimeoutError(57014, "canceling statement due to user request") on the follow-up statement. Reverting onlyPgConnection.tsand keeping the tests gives 2 failed, 16 passed inPgPool.integration.test.ts.pnpm lint,pnpm circular,pnpm check,pnpm build,pnpm check-dist-typesandpnpm test-types --target '>=5.9'all pass.pnpm ai-docgenleaves the tree clean.deno check .was not run locally; deno was unavailable on the machine used.