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

On Tue, 22 Jul 2025 17:49:49 +0900
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:

> On Fri, 18 Jul 2025 17:07:53 +0900
> Rintaro Ikeda <ikedarintarof(at)oss(dot)nttdata(dot)com> wrote:
>
> > Hi,
> >
> > On 2025/07/16 22:49, Yugo Nagata wrote:
> > >> I think we should also change the error message in pg_log_error. I modified the
> > >> patch v8-0003 as follows:
> > >> @@ -3383,8 +3383,8 @@ readCommandResponse(CState *st, MetaCommand meta, char
> > >> *varprefix)
> > >>
> > >> 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,
> > >> + pg_log_error("client %d aborted in command %d
> > >> query %d of script %d: %s",
> > >> + st->id, st->command,
> > >> qrynum, st->use_file,
> > >> PQerrorMessage(st->con));
> > >> goto error;
> > >> }
> > >>
> > >> With this change, the output now is like this:
> > >>> pgbench: error: client 0 aborted in command 1 query 0 of script 0: ERROR:
> > >> duplicate key value violates unique constraint "test_col2_key"
> > >>
> > >> I want hear your thoughts.
> > >
> > > My idea is to modify this as follows;
> > >
> > > 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));
> > > + commandFailed(st, "SQL", PQerrorMessage(st->con));
> > > goto error;
> > > }
> > >
> > > This fix is originally planned to be included in patch v8, but was missed.
> > > It is now included in the attached patch, v10.
> > >
> > > With this change, the output becomes:
> > >
> > > pgbench: error: client 0 aborted in command 0 (SQL) of script 0;
> > > ERROR: duplicate key value violates unique constraint "t2_pkey"
> > >
> > > Although there is a slight difference, the message is essentially the same as
> > > your proposal. Also, I believe the use of commandFailed() makes the code simpler
> > > and more consistent.
> > >
> > > What do you think?
> > >
> >
> > Thank you for the new patch! I think Nagata-san's v10 patch is a clear
> > improvement over my v9 patch. I'm happy with the changes.
>
> Thank you.
>
> I believe the patches implement the expected behavior, include appropriste doc and test
> modification, are in good shape overall, so if there are no objections,
> I'll mark this as Read-for-Committer.

I've updated the CF status to Ready for Committer.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>