Re: Suggestion to add --continue-client-on-abort option to pgbench - Mailing list pgsql-hackers

On Wed, 16 Jul 2025 21:35:01 +0900
Rintaro Ikeda <ikedarintarof@oss.nttdata.com> wrote:

> Hi,
> 
> On 2025/07/15 11:16, Yugo Nagata wrote:
> >> I noticed one small thing I’d like to discuss. I'm not sure that users clearly
> >> understand which aborted in the following error message, the client or the script.
> >>> pgbench: error: client 0 script 0 aborted in command ... query ...
> >>
> >> Since the code path always results in a client abort, I wonder if the following
> >> message might be clearer:
> >>> pgbench: error: client 0 aborted in script 0 command ... query ...
> > 
> > Indeed, it seems clearer to explicitly state that it is the client that
> > was aborted.
> > 
> > I've attached an updated patch that replaces the remaining message mentioned
> > above with a call to commandFailed(). With this change, the output in such
> > situations will now be:
> > 
> >  "client 0 aborted in command 0 (SQL) of script 0; ...."
> 
> Thank you for updating the patch!
> 
> When I executed a custom script that may raise a unique constraint violation, I
> got the following output:
> > pgbench: error: client 0 script 0 aborted in command 1 query 0: ERROR:
> duplicate key value violates unique constraint "test_col2_key"

I'm sorry. I must have failed to attach the correct patch in my previous post.
As a result, patch v8 was actually the same as v7, and the message in question
was not modified as intended.

> 
> 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?


> Also, let me ask one question. In this case, I directly modified your commit in
> the v8-0003 patch. Is that the right way to update the patch?

I’m not sure if that’s the best way, but I think modifying the patch directly is a
valid way to propose an alternative approach during discussion, as long as the original
patch is respected. It can often help clarify suggestions.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>