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.
