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

Rintaro Ikeda <ikedarintarof@oss.nttdata.com>

From: Rintaro Ikeda <ikedarintarof@oss.nttdata.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 'Yugo Nagata' <nagata@sraoss.co.jp>
Cc: "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-07-01T08:43:18Z
Lists: pgsql-hackers

Attachments

Hi,

I've updated the previous patch based on your feedback. Below is a summary of 
the changes from v4 to v5:

1. (v5-0001) Added documentation and removed some code paths in response to the 
comments.

2. (v5-0001) Modified the condition to transition from CSTATE_WAIT_RESULT to 
CSTATE_ERROR, instead of adding a condition in canRetryError(), which had 
enabled clients continue after its transaction failed. This is because, when the 
--continue-on-error option is set, clients do not retry failed transactions but 
start new ones.

3. (v5-0002) Renamed the enumerator TSTATUS_OTHER_ERROR, which could be 
mistakenly interpreted as being related to other SQL errors. It represents an 
unknown transaction status, so it has been renamed to TSTATUS_UNKNOWN_ERROR.


On 2025/06/14 0:24, Yugo Nagata wrote:
>              case PGRES_NONFATAL_ERROR:
>              case PGRES_FATAL_ERROR:
>                  st->estatus = getSQLErrorStatus(PQresultErrorField(res,
>                                                                     PG_DIAG_SQLSTATE));
>                  if (canRetryError(st->estatus))
>                  {
>                      if (verbose_errors)
>                          commandError(st, PQerrorMessage(st->con));
>                      goto error;
>                  }
>                  /* fall through */
> 
>              default:
>                  /* anything else is unexpected */
>                  pg_log_error("client %d script %d aborted in command %d query %d: %s",
>                               st->id, st->use_file, st->command, qrynum,
>                               PQerrorMessage(st->con));
>                  goto error;
>          }
> 
> When an SQL error other than a serialization or deadlock error occurs, an error message is
> output via pg_log_error in this code path. However, I think this should be reported only
> when verbose_errors is set, similar to how serialization and deadlock errors are handled when
> --continue-on-error is enabled

I think the error message logged via pg_log_error is useful when verbose_errors 
is not specified, because it informs users that the client has exited. Without 
it, users may not notice that something went wrong.


On 2025/06/27 19:59, Hayato Kuroda (Fujitsu) wrote:
> BTW, initially we were discussing the combination of --continue-on-error and
> --exit-on-abort. What it the conclusion?
> I feel the Nagata-san's point [1] is valid in this approach.

I agree with the conclusion. I've removed the code path that prohibited using 
--continue-on-error and --exit-on-abort options together.


On 2025/06/30 15:02, Yugo Nagata wrote:
> On Fri, 27 Jun 2025 10:59:09 +0000
> "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com> wrote:
> 
>>>> Retrying the failed transaction is not necessary when the transaction
>>>> failed due to SQL-level errors. Unlike real-world applications, pgbench
>>>> does not need to complete specific transaction successfully. In the case
>>>> of unique constraint violations, retrying the same transaction will
>>>> likely to result in the same error again.
>>
>> I intended here that clients could throw away the failed transaction and start
>> new one again in the case. I hope we are on the same page...
> 
> Could I confirm what you mean by "start new one"?
> 
> In the current pgbench, when a query raises an error (a deadlock or
> serialization failure), it can be retried using the same random state.
> This typically means the query will be retried with the same parameter values.
> 
> On the other hand, when the query ultimately fails (possibly after some retries),
> the transaction is marked as a "failure", and the next transaction starts with a
> new random state (i.e., with new parameter values).
> 
> Therefore, if a query fails due to a unique constraint violation and is retried
> with the same parameters, it will keep failing on each retry.

Thank you for your explanation. I understand it as you described. I've also 
attached a schematic diagram of the state machine. I hope it will help clarify 
the behavior of pgbench. Red arrows represent the transition of state when SQL 
command fails and --continue-on-error option is specified.

Best Regards,
Rintaro Ikeda

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.