On Fri, 7 Nov 2025 18:33:17 +0900 Fujii Masao <[email protected]> wrote:
> I plan to commit the patch soon, but let's keep discussing and > investigating the case you mentioned afterward! I'm sorry for the late reply and for not joining the discussion earlier. I've spent some time investigating the code in pgbench and libpq, and it seems to me that your commit looks fine. However, I found another issue related to the --continue-on-error option, where an assertion failure occurs in the following test case: $ cat pgbench_error.sql \startpipeline select 1/0; \syncpipeline select 1; \syncpipeline select 1; \syncpipeline select 1; \endpipeline $ pgbench -f pgbench_error.sql -M extended --continue-on-error -T 1 pgbench (19devel) starting vacuum...end. pgbench: pgbench.c:3594: discardUntilSync: Assertion `res == ((void *)0)' failed. Even after removing the Assert(), we get the following error: pgbench: error: client 0 aborted: failed to exit pipeline mode for rolling back the failed transaction This happens because discardUntilSync() does not expect that a PGRES_TUPLES_OK may be received after \syncpipeline, and also fails to discard all PGRES_PIPELINE_SYNC results when multiple \syncpipeline commands are used. I've attached a patch to fix this. 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. I think this fix should be back-patched, since this is not a bug introduced by --continue-on-error. The same assertion failure occurs in the following test case, where transactions are retried after a deadlock error: $ cat deadlock.sql \startpipeline select * from a order by i for update; select 1; \syncpipeline select 1; \syncpipeline select 1; \syncpipeline select 1; \endpipeline $ cat deadlock2.sql \startpipeline select * from a order by i desc for update; select 1; \syncpipeline select 1; \syncpipeline select 1; \syncpipeline select 1; \endpipeline $ pgbench -f deadlock.sql -f deadlock2.sql -j 2 -c 2 -M extended Regards, Yugo Nagata -- Yugo Nagata <[email protected]>
>From 6a20315b9d25ddc9f77b96d2e8318d9853b105eb Mon Sep 17 00:00:00 2001 From: Yugo Nagata <[email protected]> Date: Tue, 11 Nov 2025 10:14:30 +0900 Subject: [PATCH] Make sure discardUntilSync() discards until the last sync point --- src/bin/pgbench/pgbench.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index d8764ba6fe0..c31dd30672b 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -3563,14 +3563,14 @@ 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 since all PGRES_PIPELINE_SYNC may be already received. */ if (!PQpipelineSync(st->con)) { pg_log_error("client %d aborted: failed to send a pipeline sync", @@ -3588,10 +3588,15 @@ discardUntilSync(CState *st) else if (received_sync) { /* - * PGRES_PIPELINE_SYNC must be followed by another - * PGRES_PIPELINE_SYNC or NULL; otherwise, assert failure. + * If a PGRES_PIPELINE_SYNC is followed by something other than + * PGRES_PIPELINE_SYNC or NULL, another PGRES_PIPELINE_SYNC will + * eventually follow. */ - Assert(res == NULL); + if (res) + { + received_sync = false; + continue; + } /* * Reset ongoing sync count to 0 since all PGRES_PIPELINE_SYNC -- 2.43.0
