Re: Suggestion to add --continue-client-on-abort option to pgbench

On Tue, Sep 23, 2025 at 11:58 AM Rintaro Ikeda
<ikedarintarof(at)oss(dot)nttdata(dot)com> wrote:
> I think the issue is a new bug because we have transitioned to CSTATE_ABORT
> immediately after queries failed, without executing discardUntilSync().

If so, the fix in discardUntilSync() should be part of the continue-on-error
patch. The assertion failure fix should be a separate patch, since only
that needs to be backpatched (the failure can also occur in back branches).

> I've attached a patch that fixes the assertion error. The content of v1 patch by
> Mr. Nagata is also included. I would appreciate it if you review my patch.

+ if (received_sync == true)

For boolean flags, we usually just use the variable itself instead of
"== true/false".

+ switch (PQresultStatus(res))
+ {
+ case PGRES_PIPELINE_SYNC:
+ received_sync = true;

In the PGRES_PIPELINE_SYNC case, PQclear(res) isn't called but should be.

+ case PGRES_FATAL_ERROR:
+ PQclear(res);
+ goto done;

I don't think we need goto here. How about this instead?

-----------------------
@@ -3525,11 +3525,18 @@ discardUntilSync(CState *st)
* results have been discarded.
*/
st->num_syncs = 0;
- PQclear(res);
break;
}
+ /*
+ * Stop receiving further results if PGRES_FATAL_ERROR
is returned
+ * (e.g., due to a connection failure) before
PGRES_PIPELINE_SYNC,
+ * since PGRES_PIPELINE_SYNC will never arrive.
+ */
+ else if (PQresultStatus(res) == PGRES_FATAL_ERROR)
+ break;
PQclear(res);
}
+ PQclear(res);

/* exit pipeline */
if (PQexitPipelineMode(st->con) != 1)
-----------------------

Regards,

--
Fujii Masao