RE: Suggestion to add --continue-client-on-abort option to pgbench
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Rintaro Ikeda' <ikedarintarof@oss.nttdata.com>
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-06-09T09:34:03Z
Lists: pgsql-hackers
Dear Ikeda-san,
Thanks for updating the patch!
> 1. I should've also set benchmarking_option_set. I've modified it accordingly.
Confirmed it has been fixed. Thanks.
> 2. The exit-on-abort option and continue-on-error option are mutually exclusive.
> Therefore, I've updated the patch to throw a FATAL error when two options are
> set simultaneously. Corresponding explanation was also added.
> (I'm wondering the name of parameter should be continue-on-abort so that users
> understand the two option are mutually exclusive.)
Make sense, +1.
Here are new comments.
01. build failure
According to the cfbot [1], the documentation cannot be built. IIUC </para> seemed
to be missed here:
```
+ <para>
+ Note that this option can not be used together with
+ <option>--exit-on-abort</option>.
+ </listitem>
+ </varlistentry>
```
02. patch separation
How about separating the patch series like:
0001 - contains option handling and retry part, and documentation
0002 - contains accumulation/reporting part
0003 - contains tests.
I hope above style is more helpful for reviewers.
03. documentation
```
+ Note that this option can not be used together with
+ <option>--exit-on-abort</option>.
```
I feel we should add the similar description in `exit-on-abort` part.
04. documentation
```
+ Client rolls back the failed transaction and starts a new one when its
+ transaction fails due to the reason other than the deadlock and
+ serialization failure. This allows all clients specified with -c option
+ to continuously apply load to the server, even if some transactions fail.
```
I feel the description contains bit redundant part and misses the default behavior.
How about:
```
<para>
Clients survive when their transactions are aborted, and they continue
their run. Without the option, clients exit when transactions they run
are aborted.
</para>
<para>
Note that serialization failures or deadlock failures do not abort the
client, so they are not affected by this option.
See <xref linkend="failures-and-retries"/> for more information.
</para>
```
05. StatsData
```
+ * When continue-on-error option is specified,
+ * failed (the number of failed transactions) =
+ * 'other_sql_failures' (they got a error when continue-on-error option
+ * was specified).
```
Let me confirm one point; can serialization_failures and deadlock_failures be
counted when continue-on-error is true? If so, the comment seems not correct for me.
The formula can be 'serialization_failures' + 'deadlock_failures' + 'deadlock_failures'
in the case.
06. StatsData
Another point; can other_sql_failures be counted when the continue-on-error is NOT
specified? I feel it should be...
06. usage()
Added line is too long. According to program_help_ok(), the output by help should
be less than 80.
07.
Please run pgindent/pgperltidy, I got some diffs.
[1]: https://cirrus-ci.com/task/5210061275922432
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
pgbench: Fix assertion failure with multiple \syncpipeline in pipeline mode.
- 5bc251b28839 17.8 landed
- 00e64e35c8bd 18.2 landed
- 4aa0ac05765e 19 (unreleased) landed
-
pgbench: Add --continue-on-error option.
- 0ab208fa505c 19 (unreleased) landed
-
Fix "inconsistent DLL linkage" warning on Windows MSVC
- a3ea5330fcf4 19 (unreleased) cited
-
pgbench: Fix error reporting in readCommandResponse().
- 7d7e81d1e6b8 13.23 landed
- cb21e1f92fb3 14.20 landed
- bdccb6302871 15.15 landed
- 36c4d30c8f24 16.11 landed
- a912118c6055 17.7 landed
- 29aabbc43269 18.1 landed
- 19d4f9ffc207 19 (unreleased) landed
-
pgbench: Fix assertion failure with retriable errors in pipeline mode.
- 704f517711b4 15.15 landed
- 8b2e290bde21 16.11 landed
- f39d9164b70a 17.7 landed
- c736808e03b7 18.1 landed
- 8bb174295e89 19 (unreleased) landed
-
Allow pgbench to retry in some cases.
- 4a39f87acd6e 15.0 cited