Query Cancellation Design for ponylang/postgres #88
Replies: 5 comments
-
|
Two design decisions made during review: 1. Accessor naming: The Session accessor methods should be named for what they are, not what they're used for. Use 2. SSL support included, phased implementation: The initial design deferred SSL on the cancel connection. Decision: SSL support will be included in the same PR, but implemented in phases — get non-SSL cancel working first as a checkpoint, then add SSL support to |
Beta Was this translation helpful? Give feedback.
-
|
Missing fun ref _on_received(data: Array[U8] iso) =>
None |
Beta Was this translation helpful? Give feedback.
-
|
SSL support needs to be part of the main design: the comment decision says SSL is included in the same PR, but the main design body doesn't account for this —
Related: the SSL-capable |
Beta Was this translation helpful? Give feedback.
-
Implementation PlanPhased implementation plan for query cancellation, incorporating the two design decisions from previous comments (generic accessor names, SSL included). This is "two PRs in one" — Phase 1 (non-SSL cancel) is implemented and reviewed at a checkpoint before Phase 2 (SSL cancel) is added on top. Final PR is a single squashed commit. PrerequisitesCreate feature branch Phase 1: Non-SSL CancelStep 1 — Step 2 — Session fields and accessors: Store Step 3 — Step 4 — State machine: Add Step 5 — Session Step 6 — Tests:
Step 7 — Example: Step 8 — CLAUDE.md: Update architecture, protocol, file layout, test organization sections. Checkpoint: Squash, push, run reviewer subagent. Fix findings. Iterate until clean. Phase 2: SSL CancelStep 9 — SSL mode on Session: Add Step 10 — Upgrade
Update Step 11 — SSL cancel tests:
Step 12 — CLAUDE.md: Document SSL support in Final StepsReviewer subagent against all changes. Fix findings. Squash all commits into single commit. Open PR with Verification |
Beta Was this translation helpful? Give feedback.
-
|
Implemented in PR #89 (merged). Both non-SSL and SSL cancel paths are complete. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Problem
PostgreSQL query cancellation requires sending a
CancelRequeston a separate TCP connection from the one executing the query. The currentSessionarchitecture uses a single actor with a single TCP connection, so cancellation requires a fundamentally different internal approach than other operations, which all go through the existing connection.PostgreSQL CancelRequest Protocol
BackendKeyDatawithprocess_idandsecret_key(already parsed and stored in_SessionLoggedIn.backend_pid/_SessionLoggedIn.backend_secret_key)CancelRequest(16, 80877102, process_id, secret_key), close the connectionErrorResponsewith SQLSTATE57014(query_canceled), thenReadyForQuery80877102 = 1234 << 16 | 5678Design
New Actor:
_CancelSenderA small, internal, fire-and-forget actor in a new file
_cancel_sender.pony. It:lori.TCPConnectionActor & lori.ClientLifecycleEventReceiver_on_connected(): sends the CancelRequest message then closes the connection_on_connection_failure(): silently does nothing (fire-and-forget)New Frontend Message:
cancel_requestAdded to the
_FrontendMessageprimitive:Session Changes
New fields on Session actor — immutable connection parameters already passed to the constructor, now stored for use by cancel:
New package-accessible methods on Session (no underscore prefix, so other types in the package can access them):
New behavior on Session:
State Machine Changes
_SessionStateinterface — add:Default implementations via trait hierarchy:
_UnconnectedState: addcancelas no-op — covers_SessionUnopened,_SessionClosed_ConnectedState: addcancelas no-op — covers_SessionConnected(via_AuthenticableState) and provides default for_SessionLoggedIn(via_AuthenticatedState, which the class overrides)_SessionSSLNegotiating: add explicitcancelas no-op (standalone class, doesn't mix in the above)_SessionLoggedIn.cancel()— the real implementation:The match checks whether a query is actually in flight.
_QueryReady(idle) and_QueryNotReady(waiting for readiness) mean no query is executing on the server. Any other_QueryState(_SimpleQueryInFlight,_ExtendedQueryInFlight,_PrepareInFlight,_CloseStatementInFlight) means something is in flight.Note on timing: due to Pony's actor model,
cancel()is processed asynchronously. By the time it runs, the query may have already completed and the state may have transitioned. This is fine — a stale CancelRequest is harmless (the server ignores cancels for processes that aren't running a query). The check is a best-effort optimization to avoid unnecessary TCP connections.What This Design Does NOT Change
on_error_responsepath. TheResultReceivergetspg_query_failed(session, query, ErrorResponseMessage)where the ErrorResponseMessage has SQLSTATE57014. No new error types are needed — the server's ErrorResponse already fully describes the cancellation.cancel()only targets the in-flight query on the server. Queued queries remain and execute normally after cancellation completes.ReadyForQueryarrives normally on the original connection and triggers the usual transition to_QueryReady, which dequeues the next query.SSL Considerations
Initial implementation:
_CancelSenderuses a plain TCP connection regardless of whether the original session uses SSL. CancelRequest contains no sensitive data (just pid + key that were already exchanged over the secure channel), so the security risk is minimal.Limitation: If the PostgreSQL server is configured with
hostsslentries that reject all non-SSL connections, the cancel connection will be silently refused. This is documented.Future enhancement: Add SSL support to
_CancelSenderby:SSLModefrom the original session to_CancelSenderSSLRequired: send SSLRequest, wait for 'S', TLS handshake, then CancelRequest_CancelSendera mini state machine rather than purely fire-and-forgetSSLContext valfrom the original session can be reused (it'sval, freely shareable)This SSL enhancement could be a follow-up PR. The non-SSL cancel covers the common deployment case (mixed SSL/non-SSL acceptance, or non-SSL deployments).
File Changes
_cancel_sender.pony_CancelSenderactor_frontend_message.ponycancel_request(process_id, secret_key)methodsession.pony_auth,_host,_service; addcancel()behavior; addcancelto_SessionStateinterface and trait defaults; implement_SessionLoggedIn.cancel()_test_frontend_message.ponycancel_request()wire format_test.pony_test_query.ponypg_sleep()query, verifypg_query_failedwith SQLSTATE 57014examples/cancel/cancel-example.ponyCLAUDE.mdcancel()to Session behaviors in Query Execution Flow section; (2) add_CancelSenderandcancel_request()to Architecture/Protocol sections; (3) add_cancel_sender.ponyandexamples/cancel/to File Layout; (4) add cancel-related tests to Test OrganizationBuilding and Running Tests
Design Decisions and Rationale
Cancel on Session, not standalone: Cancel is a method on Session because Session already holds the backend key data and connection parameters. A standalone API would require exposing
backend_pid/backend_secret_keypublicly, leaking internal protocol details.No new error types: The server already provides a fully descriptive ErrorResponse for cancellation (SQLSTATE 57014). Adding a client-side
QueryCancelledtype would be redundant.No queue draining:
cancel()only targets the server-side in-flight query. Cancelling queued-but-not-sent queries is a different concern (queue management, not PostgreSQL cancellation protocol). If needed, a separateclear_queue()API could be added independently.Fire-and-forget
_CancelSender: No callback or notification about whether the cancel connection succeeded. This matches the PostgreSQL protocol semantics — the result is always delivered via the original connection's ErrorResponse/CommandComplete flow.Check before sending: Although a stale CancelRequest is harmless, we check
query_stateto avoid creating an unnecessary TCP connection + actor when we already know the server is idle.Beta Was this translation helpful? Give feedback.
All reactions