Re: pgbench: allow to exit immediately when any client is aborted - Mailing list pgsql-hackers

From Yugo NAGATA
Subject Re: pgbench: allow to exit immediately when any client is aborted
Date
Msg-id 20230808094747.c4f4a2da07f6202a6cdec323@sraoss.co.jp
Whole thread Raw
In response to Re: pgbench: allow to exit immediately when any client is aborted  (Fabien COELHO <coelho@cri.ensmp.fr>)
Responses Re: pgbench: allow to exit immediately when any client is aborted
List pgsql-hackers
Hello Fabien,

On Mon, 7 Aug 2023 12:17:38 +0200 (CEST)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

> 
> Hello Yugo-san,
> 
> > I attached v2 patch including the documentation and some comments
> > in the code.
> 
> I've looked at this patch.

Thank you for your review!

> 
> I'm unclear whether it does what it says: "exit immediately on abort", I 
> would expect a cold call to "exit" (with a clear message obviously) when 
> the abort occurs.
> 
> Currently it skips to "done" which starts by closing this particular 
> thread client connections, then it will call "exit" later, so it is not 
> "immediate".

There are cases where "goto done" is used where some error like
"invalid socket: ..." happens. I would like to make pgbench exit in
such cases, too, so I chose to handle errors below the "done:" label.
However, we can change it to call "exit" instead of "goo done" at each
place. Do you think this is better?
 
> I do not think that this cleanup is necessary, because anyway all other 
> threads will be brutally killed by the exit called by the aborted thread, 
> so why bothering to disconnect only some connections?

Agreed. This disconnection is not necessary anyway even when we would like
to handle it below "done".

>      /* If any client is aborted, exit immediately. */
>      if (state[i].state != CSTATE_FINISHED)
> 
> For this comment, I would prefer "if ( ... == CSTATE_ABORTED)" rather that 
> implying that not finished means aborted, and if you follow my previous 
> remark then this code can be removed.

Ok. If we handle errors like "invalid socket:" (mentioned above) after
skipping to "done", we should set the status to CSTATE_ABORTED before the jump. 
Otherwise, if we choose to call "exit" immediately at each error instead of
skipping to "done", we can remove this as you says.

> Typo: "going to exist" -> "going to exit". Note that it is not "the whole 
> thread" but "the program" which is exiting.

I'll fix.
 
> There is no test.

I'll add an test.

Regards,
Yugo Nagata

 
> -- 
> Fabien.


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



pgsql-hackers by date:

Previous
From: Masahiro Ikeda
Date:
Subject: Re: Support to define custom wait events for extensions
Next
From: Richard Guo
Date:
Subject: Re: Check volatile functions in ppi_clauses for memoize node