Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Mon, 19 Jun 2023 16:49:05 -0700
Message-Id: <CTH123B82H6F.34NSTLQ0XE4AN@gonk>
To: "Yugo NAGATA" <nagata@sraoss.co.jp>
From: "Tristan Partin" <tristan@neon.tech>
Cc: "Michael Paquier" <michael@paquier.xyz>, <pgsql-hackers@postgresql.org>
Subject: Re: Make pgbench exit on SIGINT more reliably
References: <CSSWBAX56CVY.291H6ZNNHK7EO@c3po> <ZG1a5lsSXXiYVJQ6@paquier.xyz>
 <CSUK7YEFYAFZ.3P26BKFUDTO06@c3po>
 <20230619223937.209ff5c4c295da2fbb4adaa0@sraoss.co.jp>
In-Reply-To: <20230619223937.209ff5c4c295da2fbb4adaa0@sraoss.co.jp>
Archived-At: <https://www.postgresql.org/message-id/CTH123B82H6F.34NSTLQ0XE4AN%40gonk>
Precedence: bulk

On Mon Jun 19, 2023 at 6:39 AM PDT, Yugo NAGATA wrote:
> On Wed, 24 May 2023 08:58:46 -0500
> "Tristan Partin" <tristan@neon.tech> wrote:
>
> > On Tue May 23, 2023 at 7:31 PM CDT, Michael Paquier wrote:
> > > On Mon, May 22, 2023 at 10:02:02AM -0500, Tristan Partin wrote:
> > > > The way that pgbench handled SIGINT changed in
> > > > 1d468b9ad81b9139b4a0b16b416c3597925af4b0. Unfortunately this had a
> > > > couple of unintended consequences, at least from what I can tell[1]=
.
> > > >=20
> > > > - CTRL-C no longer stops the program unless the right point in pgbe=
nch
> > > >   execution is hit=20
> > > > - pgbench no longer exits with a non-zero exit code
> > > >=20
> > > > An easy reproduction of these problems is to run with a large scale
> > > > factor like: pgbench -i -s 500000. Then try to CTRL-C the program.
> > >
> > > This comes from the code path where the data is generated client-side=
,
> > > and where the current CancelRequested may not be that responsive,
> > > isn't it?
> >=20
> > It comes from the fact that CancelRequested is only checked within the
> > generation of the pgbench_accounts table, but with a large enough scale
> > factor or enough latency, say cross-continent communication, generating
> > the other tables can be time consuming too. But, yes you are more likel=
y
> > to run into this issue when generating client-side data.
>
> If I understand correctly, your patch allows to exit pgbench immediately
> during a client-side data generation even while the tables other than
> pgbench_accounts are processed. It can be useful when the scale factor
> is large. However, the same purpose seems to be achieved by you other pat=
ch [1].
> Is the latter your latest proposal that replaces the patch in this thread=
?ad?
>
> [1] https://www.postgresql.org/message-id/flat/CSTU5P82ONZ1.19XFUGHMXHBRY=
%40c3po

The other patch does not replace this one. They are entirely separate.

> Also, your proposal includes to change the exit code when pgbench is canc=
elled by
> SIGINT. I think it is nice because this will make it easy to understand a=
nd clarify=20
> the result of the initialization.=20
>
> Currently, pgbench returns 0 when the initialization is cancelled while p=
opulating
> pgbench_branches or pgbench_tellers, but the resultant pgbench_accounts h=
as only
> one row, which is an inconsistent result. Returning the specific value fo=
r SIGINT
> cancel can let user know it explicitly. In addition, it seems better if a=
n error
> message to inform would be output.=20
>
> For the above purpose, the patch moved exit codes of psql to fe_utils to =
be shared.
> However,  I am not sure this is good way. Each frontend-tool may have it =
own exit code,
> for example. "2" means "bad connection" in psql [2], but "errors during t=
he run" in
> pgbench [3]. So, I think it is better to define them separately.
>
> [2] https://www.postgresql.org/docs/current/app-psql.html#id-1.9.4.20.7
> [3] https://www.postgresql.org/docs/current/pgbench.html#id=3Did-1.9.4.11=
.7

I can change it. I just figured that sharing exit code definitions
would've been preferrable. I will post a v3 some time soon which removes
that patch.

Thanks for your review :).

--=20
Tristan Partin
Neon (https://neon.tech)