Re: pgbench exit code - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: pgbench exit code
Date
Msg-id alpine.DEB.2.21.1808100110080.9208@lancre
Whole thread Raw
In response to pgbench exit code  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: pgbench exit code
Re: pgbench exit code
List pgsql-hackers
Hello Peter,

> In pgbench, when an error occurs during a benchmark run (SQL error,
> connection error, etc.) it just prints an error message and then
> proceeds to print the run summary normally and exits with status 0.  So
> this is quite inconvenient, especially from a script.

Yep. I'm fine with changing this behavior... last time I suggested 
something like that, or probably something more drastic like existing 
immediately when there is little sense to go on, it was not approved, some 
people want the report anyway.

Your approach of not changing the output too much but changing the exit 
status and adding a warning may get through more easily.

Note that there is some logic in distinguishing between different type of 
errors (before the bench start vs the bench has started), so I'd suggest 
that the exit status should be different.

> The attached patch changes this so that it exits with status 1 and also
> prints a line below the summary advising that the results are incomplete.
>
> The test suite had, curiously, encoded the previous behavior,

Sure, the point of the tests is to check for unwanted regressions. It does 
not mean that the behavior is ideal for all use cases.

Patch applies cleanly, compiles, global & local "make check" are ok.

The abort is by a client, but the code seems to only check the first 
client of a thread. ISTM that if the second or later client abort it may 
not be detected? Probably an intermediate aggregation at the thread level 
is needed, or maybe a global variable, or as errors are counted somewhere, 
it may be enough just to check that the count is non zero?

  -               [ $status ? qr{^$} : qr{processed: 0/1} ],
  +               [],


The patch removes a check that there was an output and that no 
transactions where processed. ISTM it should be kept. If a different exit 
status is chosen on abort, that would allow to keep it easily.

Probably there should be some documentation changes as well?

Note that the patch probably interferes in small ways with Marina's patch 
for retrying on serialization or deadlock errors, see:

     https://commitfest.postgresql.org/18/1645/

-- 
Fabien.


pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: Commitfest 2018-07 RFC items
Next
From: Tom Lane
Date:
Subject: Re: CF 2018-07 Needs Review items