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

On Tue, Sep 23, 2025 at 11:58 AM Rintaro Ikeda
<ikedarintarof@oss.nttdata.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