Skip to content

postgres: send no Close of the unnamed portal before Sync - #4464

Open
airhorns wants to merge 1 commit into
transact-rs:mainfrom
airhorns:postgres-no-close-before-sync
Open

airhorns wants to merge 1 commit into
transact-rs:mainfrom
airhorns:postgres-no-close-before-sync

Conversation

@airhorns

Copy link
Copy Markdown

Does your PR solve an issue?

No issue is open for this. It follows up #2081, the fix for #2077, whose Close this change removes now that #3673 makes every Execute run to completion.

Is this a breaking change?

No. The client sends the same statements and reads the same results. The only change is that the unnamed portal of a completed statement is destroyed by the next Bind or the end of the transaction instead of by an explicit Close, which is what libpq does. it_runs_statements_after_fetch_one_in_a_transaction_issue_2077 checks that #2077 stays fixed.

PgConnection::run sends Bind, Execute, Close of the unnamed portal, and Sync for every statement
with arguments. This change drops the Close, so the sequence is Bind, Execute, Sync, which is what
libpq sends for PQexecParams.

Why

YugabyteDB decides whether an Execute is part of a batch by looking at the message that follows it.
With the default yb_pg_batch_detection_mechanism (detect_by_peeking), an Execute that is
followed by any message other than Sync counts as batched. In READ COMMITTED, YugabyteDB normally
reruns a statement that hit a conflict at the query layer, but not a batched statement once the
transaction has already written: yb_is_retry_possible in src/postgres/src/backend/tcop/postgres.c
refuses the retry when YbIsBatchedExecution() && GetCurrentCommandId(false) > FirstCommandId.

Because sqlx puts a Close between Execute and Sync, every statement that runs after a transaction's
first write is treated as batched. A conflict on such a statement reaches the application as
SQLSTATE 40001:

could not serialize access due to concurrent update (query layer retries aren't supported when
executing non-first statement in batch, will be unable to replay earlier commands)

Through libpq, the same transactions get YugabyteDB's retry and see no error.

Why the Close is no longer needed

The Close came with #2081, the fix for #2077. At the time, fetch_one and fetch_optional
executed the portal with a row limit of 1. The portal stopped after one row (PortalSuspended) and
stayed open, and CockroachDB rejected the next statement in the transaction with "multiple active
portals not supported".

Since #3673, every Execute is sent with a row limit of 0, so the portal always runs to completion
and is never suspended. The PostgreSQL protocol documentation says the unnamed portal "is destroyed
at the end of the transaction, or as soon as the next Bind statement specifying the unnamed portal
as destination is issued", and a simple Query also destroys it. The Close therefore no longer
prevents a suspended portal. It only frees a completed portal one statement earlier.

The cost of this change: the completed unnamed portal of a transaction's last statement now lives
until the next statement or the end of the transaction, as it does with libpq.

Measurement

YugabyteDB 2025.2.5.2, single node, READ COMMITTED. Six tasks each ran ten transactions. Each
transaction first updated one shared row and then ran the step-completion statements of a
durable-workflow library, which lock a row that a concurrent lease renewal also writes. We ran two
copies at once, three runs each, and counted the 40001 errors the library caught and retried:

sqlx-postgres Transactions per run Runs with 40001 40001 per run
0.8.6 as published 60 6 of 6 20, 2, 4, 6, 14, 5
0.8.6 without the Close 60 0 of 6 0

The other contention cases in the same runs (claims, renewals, and the same completions without
the earlier write) behaved the same with and without the Close. An earlier run on the same setup
gave 3, 11, and 11 conflicts with the Close and 0 without it. Through libpq (ysql_bench, six
clients), transactions of the same kind, a write followed by a locking read that waits for another
writer, met no serialization failure.

Test

it_runs_statements_after_fetch_one_in_a_transaction_issue_2077 covers #2077's case. In one
transaction it reads one row of a 50-row result with fetch_one three times, then creates a
temporary table, inserts into it, reads one row of it with fetch_one, counts the rows, and
commits. It passes on PostgreSQL and on YugabyteDB.

The test does not assert that no Close is sent. Asserting the message sequence needs a proxy that
reads the protocol, and the client-certificate CI job only accepts TLS connections, which such a
proxy cannot read.

馃 Generated with Claude Code

PgConnection::run sent Bind, Execute, Close of the unnamed portal, and
Sync for every statement with arguments. Send Bind, Execute, and Sync,
as libpq does.

The Close came with transact-rs#2081 for transact-rs#2077, when fetch_one executed the
portal with a row limit and left it suspended. Since transact-rs#3673 every
Execute has no row limit, so the portal runs to completion, and the
next Bind or the end of the transaction destroys it.

YugabyteDB treats an Execute that another message follows before the
Sync as part of a batch, and does not retry a conflicting statement of
a batch once its transaction has written. With the Close, such
conflicts reached the application as SQLSTATE 40001.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant