Design: Transaction Status Tracking #102
Closed
SeanTAllen
started this conversation in
Research
Replies: 2 comments 1 reply
-
|
Is |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
Both parts of this design are now implemented:
Closing as fully implemented. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Expose the PostgreSQL transaction status indicator (ReadyForQuery status byte) to users via a new callback on
SessionStatusNotify, and fix a bug where explicit transactions (BEGIN/COMMIT) stall the query queue.This is a multi-type PR: a bug fix (broken explicit transactions) and a new feature (transaction status callback). Per CHANGELOG procedures, this needs manual CHANGELOG handling with entries under both "Fixed" and "Added".
Background
Every
ReadyForQuerymessage includes a status byte:I— Idle (not in a transaction)T— In a transaction block (afterBEGIN, beforeCOMMIT/ROLLBACK)E— In a failed transaction block (queries rejected untilROLLBACK)The driver already parses this byte —
_ReadyForQueryMessagehasidle(),in_transaction_block(), andfailed_transaction()methods — but does not expose it to users.Bug Found During Research
The query state machine only transitions from
_QueryNotReadyto_QueryReadywhen the status is idle ('I'). When the status is'T'or'E', the state stays_QueryNotReady, which prevents the queue from dispatching the next operation. This means explicit transactions are broken:BEGIN— dispatched via_QueryReady, enters_SimpleQueryInFlight'T')on_ready_for_querychecksmsg.idle()— false — transitions to_QueryNotReadyThe same
msg.idle()check exists in all five query states:_QueryNotReady,_SimpleQueryInFlight,_ExtendedQueryInFlight,_PrepareInFlight,_CloseStatementInFlight.This fix is included in this design because transaction status tracking is only useful when transactions actually work.
Design
New Public Types
transaction_status.pony:New Callback
Add to
SessionStatusNotify:Non-breaking — the default body means existing implementations don't need to change.
Callback Delivery Point
In
_SessionLoggedIn.on_ready_for_query, before delegating to the query state:The conversion from status byte to
TransactionStatusis done by a new method on_ReadyForQueryMessage:This replaces the three individual methods (
idle(),in_transaction_block(),failed_transaction()) which become unused after the query state machine fix.Message Ordering
When
SessionStatusNotifyandResultReceiverare implemented by the same actor,pg_query_resultis always delivered beforepg_transaction_statusbecauseCommandCompleteprecedesReadyForQueryin the protocol. The same holds forpg_query_failed(from ErrorResponse). When implemented by different actors, the sends are ordered but delivery depends on actor scheduling.Bug Fix: Always Transition to _QueryReady on ReadyForQuery
ReadyForQuery means the server is ready for the next command — the status byte is informational, not a readiness indicator. Change every
on_ready_for_queryin the query state machine to always transition to_QueryReady.Since the query states no longer inspect the
_ReadyForQueryMessage, remove themsgparameter from the_QueryState.on_ready_for_queryinterface. The_SessionState.on_ready_for_queryinterface retainsmsg(it's called from_ResponseMessageParserwhich passes it). All five_QueryStateimplementations simplify:_QueryNotReady— before:After:
Update
_QueryNotReadydocstring: it is now only the initial state after authentication (before the first ReadyForQuery). It is no longer entered from non-idle ReadyForQuery responses._SimpleQueryInFlight,_ExtendedQueryInFlight,_PrepareInFlight,_CloseStatementInFlight— all have the same change pattern. Before (using_SimpleQueryInFlightas example):After:
_QueryReady(protocol anomaly handler) — unchanged in behavior, just drops the unused parameter.Files Changed
postgres/transaction_status.pony—TransactionIdle,TransactionInBlock,TransactionFailed,TransactionStatuspostgres/session_status_notify.pony— addpg_transaction_statuscallbackpostgres/_backend_messages.pony— replaceidle(),in_transaction_block(),failed_transaction()withtransaction_status()on_ReadyForQueryMessage; update docstringpostgres/session.pony— fire callback in_SessionLoggedIn.on_ready_for_query; removemsgparameter from_QueryState.on_ready_for_queryinterface; removemsg.idle()checks from all five query states; update_QueryNotReadydocstringpostgres/_test_response_parser.pony— update parser tests that callidle(),in_transaction_block(),failed_transaction()to usetransaction_status()insteadpostgres/_test.pony— register new test classes inMaintest actorCLAUDE.md— update architecture (Query Execution Flow description of_QueryNotReady), test organization (new_test_transaction_status.ponyentry), public API types (TransactionStatus), file layout (new files), supported featuresTests
Unit Tests (Mock Servers)
New test file:
postgres/_test_transaction_status.ponyMock port assignments: 7683, 7684, 7685
_TestTransactionStatusIdle(port 7683) — mock server sends AuthOk + ReadyForQuery('I'). Client verifiespg_transaction_statusfires withTransactionIdle._TestTransactionStatusInBlock(port 7684) — mock server authenticates, then on query sends CommandComplete + ReadyForQuery('T'). Client sends a second query to verify the queue dispatches (the bug fix). Verifiespg_transaction_statusfires withTransactionInBlock. The test client tracks the sequence of status callbacks to verify the expected order._TestTransactionStatusFailed(port 7685) — mock server authenticates, then on query sends ErrorResponse + ReadyForQuery('E'). Client sends a follow-up query (ROLLBACK simulation) to verify the queue still dispatches. Verifiespg_transaction_statusfires withTransactionFailed. The test client tracks the sequence of status callbacks.Integration Tests
Added to
postgres/_test_query.pony:_TestTransactionCommit— BEGIN, INSERT into a test table, COMMIT. The test client tracks the sequence ofpg_transaction_statuscallbacks and verifies the transitions: Idle -> InBlock -> InBlock -> Idle (four callbacks total: initial post-auth, after BEGIN, after INSERT, after COMMIT)._TestTransactionRollbackAfterFailure— BEGIN, run an invalid query (e.g., SELECT from nonexistent table), ROLLBACK. The test client tracks callbacks and verifies transitions: Idle -> InBlock -> Failed -> Idle.Existing Tests
No regressions expected. Existing tests don't use explicit transactions, so all ReadyForQuery messages are
'I'. Theon_ready_for_querybehavior change (always transition to_QueryReady) is transparent when status is idle — the removedelsebranch was dead code in practice.Parser tests that assert on
idle(),in_transaction_block(),failed_transaction()will be updated to assert ontransaction_status()returning the correct primitive.Example
New example:
examples/transaction/transaction-example.ponyDemonstrates:
pg_transaction_statuscallbackBuild/Run Commands
Beta Was this translation helpful? Give feedback.
All reactions