Re: pgbench: allow to exit immediately when any client is aborted

Hello Fabien,

On Mon, 7 Aug 2023 12:17:38 +0200 (CEST)
Fabien COELHO <coelho(at)cri(dot)ensmp(dot)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(at)sraoss(dot)co(dot)jp>