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

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: Rintaro Ikeda <ikedarintarof(at)oss(dot)nttdata(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "slpmcf(at)gmail(dot)com" <slpmcf(at)gmail(dot)com>, "boekewurm+postgres(at)gmail(dot)com" <boekewurm+postgres(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: Suggestion to add --continue-client-on-abort option to pgbench
Date: 2025-07-22 08:49:49
Message-ID: 20250722174949.83f2200bdb6cc667131d11a8@sraoss.co.jp
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Regards,
Yugo Nagata

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2025-07-22 08:59:41 Re: track needed attributes in plan nodes for executor use
Previous Message Shinya Kato 2025-07-22 08:48:35 Add backup_type to pg_stat_progress_basebackup