Horton
Menu

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

Fujii Masao <masao.fujii@gmail.com>

View thread
From: Fujii Masao <masao.fujii@gmail.com>
To: Rintaro Ikeda <ikedarintarof@oss.nttdata.com>
Cc: Yugo Nagata <nagata@sraoss.co.jp>, Jakub Wartak <jakub.wartak@enterprisedb.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "slpmcf@gmail.com" <slpmcf@gmail.com>, "boekewurm+postgres@gmail.com" <boekewurm+postgres@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Srinath Reddy Sadipiralla <srinath2133@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>
Date: 2025-09-24T17:19:27Z
Lists: pgsql-hackers 
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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. pgbench: Fix assertion failure with multiple \syncpipeline in pipeline mode.

  2. pgbench: Add --continue-on-error option.

  3. Fix "inconsistent DLL linkage" warning on Windows MSVC

  4. pgbench: Fix error reporting in readCommandResponse().

  5. pgbench: Fix assertion failure with retriable errors in pipeline mode.

  6. Allow pgbench to retry in some cases.