Pgbench: remove synchronous prepare

[pg-hub-mirror]

Read-only mirror. Reply and review on pgsql-hackers; activity here is not sent upstream.

• Original author: Dmitrii Bondar <d(dot)bondar(at)postgrespro(dot)ru>
• Mailing list: pgsql-hackers
• Message-ID: 9ac169f5-5c83-4980-bd28-27e4fe682453@postgrespro.ru
• Original email

Patch files:

• v5-0001-Remove-synchronous-prepare-from-pgbench.patch

---

Hi, Andrey!

Returning to CSTATE_START_COMMAND after preparing resets stmt_begin.
This drops the prepare time from --report-per-command. In a local test,
blocking Prepare on a table lock gave 4436 ms transaction latency
but only 0.401 ms statement latency. Could we send the query without
resetting the command timer?
Removed a transfer to CSTATE_START_COMMAND and just used sendCommand
from the new state.
The second PQgetResult() in CSTATE_WAIT_PREPARE_RESULT can still block:
the first returns ParseComplete, but the next must wait for
ReadyForQuery if it has not arrived yet. The error path also drains
results synchronously through getSQLErrorStatus(). Similar reads
already exist elsewhere in pgbench, but this leaves a blocking window
in the new prepare path. Could we wait for completion through the
event loop? A connection failure between the two results can also
make the second call return an error rather than the asserted NULL.
Spit the state into two stages to avoid blocking. Added an event loop to
the error path.
I also found a way to test the main fix without PgBouncer. With
-c 2 -j 1, use client_id to make client 0 select from a table locked
by another session in ACCESS EXCLUSIVE mode, and client 1 insert into
a different table. In local runs, client 1 made progress while
client 0 was blocked in Prepare with v4, but not without the patch.
This would test the intended behavior more directly than the changed
syntax-error expectation.
Implemented.
I am fine with leaving prepareCommandsInPipeline() for a separate patch.
Could the commit message describe this as a scope choice? Preparing
before PQenterPipelineMode() preserves the required ordering, but does
not require blocking the worker thread. The prepared-flag fix is also
limited to the non-pipeline path: prepareCommand() still sets the flag
after an error.
Fixed the flag and added a note to the commit message.
Looks much better now, thank you for reviewing the patch!
--
Regards,
Dmitrii Bondar.

[pg-hub-mirror]

Earlier design discussion: Discussion #228