On Wed, 12 Nov 2025 00:20:15 +0900 Fujii Masao <[email protected]> wrote:
> On Tue, Nov 11, 2025 at 10:50 AM Yugo Nagata <[email protected]> wrote: > > I've attached a patch to fix this. > > Thanks for reporting the issue and providing the patch! > > > If a PGRES_PIPELINE_SYNC is followed by something other than > > PGRES_PIPELINE_SYNC or NULL, > > it means that another PGRES_PIPELINE_SYNC will eventually follow after some > > other results. > > In this case, we should reset the receive_sync flag and continue discarding > > results. > > Yes. > > + if (res) > + { > + received_sync = false; > + continue; > > Shouldn't we also call PQclear(res) here? For example: Thank you for your review! Yes, we need PQclear() here. I've attached an updated patch. The comment for the PQpipelineSync() call has been also updated to clarify why it is necessary. In addition, I added a connection status check in the loop to avoid an infinte loop that waiting for PQpipelineSync after a connection failure. I packed these changes in the same patch, but they can be split into separate patches. What do you think? Regards, Yugo Nagata -- Yugo Nagata <[email protected]>
>From 8170406337210755442eedcf6b253031e22297e1 Mon Sep 17 00:00:00 2001 From: Yugo Nagata <[email protected]> Date: Tue, 11 Nov 2025 10:14:30 +0900 Subject: [PATCH v2] pgbench: Fix assertion failure with multiple \syncpipeline in pipeline mode. When running pgbench with a custom script that triggered retriable errors (e.g., deadlock errors) followed by multiple \syncpipeline commands in pipeline mode, an assertion failure could occur: pgbench.c:3594: discardUntilSync: Assertion `res == ((void *)0)' failed This happened because discardUntilSync() did not expect that a result other than NULL (e.g. PGRES_TUPLES_OK) might be received after \syncpipeline. This commit fixes the assertion failure by resetting the receive_sync flag and continueing to discard results to ensure that all results are discarded until the last sync point. Also, if the connection was unexpectedly closed, this function could get stuck in an infinite loop waiting for PGRES_PIPELINE_SYNC, which would never be received. To fix this, exit the loop immediately if a connection failure is detected. --- src/bin/pgbench/pgbench.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index d8764ba6fe0..f165fabce36 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -3563,14 +3563,18 @@ doRetry(CState *st, pg_time_usec_t *now) } /* - * Read results and discard it until a sync point. + * Read and discard results until the last sync point. */ static int discardUntilSync(CState *st) { bool received_sync = false; - /* send a sync */ + /* + * Send a sync to ensure at least one PGRES_PIPELINE_SYNC is received + * and to avoid an infinite loop, since all earlier ones may have + * already been received. + */ if (!PQpipelineSync(st->con)) { pg_log_error("client %d aborted: failed to send a pipeline sync", @@ -3578,21 +3582,15 @@ discardUntilSync(CState *st) return 0; } - /* receive PGRES_PIPELINE_SYNC and null following it */ + /* receive the last PGRES_PIPELINE_SYNC and null following it */ for (;;) { PGresult *res = PQgetResult(st->con); if (PQresultStatus(res) == PGRES_PIPELINE_SYNC) received_sync = true; - else if (received_sync) + else if (received_sync && res == NULL) { - /* - * PGRES_PIPELINE_SYNC must be followed by another - * PGRES_PIPELINE_SYNC or NULL; otherwise, assert failure. - */ - Assert(res == NULL); - /* * Reset ongoing sync count to 0 since all PGRES_PIPELINE_SYNC * results have been discarded. @@ -3601,6 +3599,23 @@ discardUntilSync(CState *st) PQclear(res); break; } + else + { + if (PQstatus(st->con) == CONNECTION_BAD) + { + pg_log_error("client %d aborted: the backend died while rolling back the failed transaction after", + st->id); + PQclear(res); + return 0; + } + + /* + * If a PGRES_PIPELINE_SYNC is followed by something other than + * PGRES_PIPELINE_SYNC or NULL, another PGRES_PIPELINE_SYNC will + * eventually follow. + */ + received_sync = false; + } PQclear(res); } -- 2.43.0
