On Wed, 24 May 2023 08:58:46 -0500 "Tristan Partin" <tris...@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]. > > > > > > - CTRL-C no longer stops the program unless the right point in pgbench > > > execution is hit > > > - pgbench no longer exits with a non-zero exit code > > > > > > 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? > > 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 likely > 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 patch [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 Also, your proposal includes to change the exit code when pgbench is cancelled by SIGINT. I think it is nice because this will make it easy to understand and clarify the result of the initialization. Currently, pgbench returns 0 when the initialization is cancelled while populating pgbench_branches or pgbench_tellers, but the resultant pgbench_accounts has only one row, which is an inconsistent result. Returning the specific value for SIGINT cancel can let user know it explicitly. In addition, it seems better if an error message to inform would be output. 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 the 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=id-1.9.4.11.7 Regards, Yugo Nagata > -- > Tristan Partin > Neon (https://neon.tech) > > -- Yugo NAGATA <nag...@sraoss.co.jp>
