Re: Some code cleanup for pgbench and pg_verifybackup

Fabien COELHO <coelho@cri.ensmp.fr>

From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-07-27T04:36:15Z
Lists: pgsql-hackers
Bonjour Michaël-san,

>>> The semantics for fatal vs error is that an error is somehow handled while a
>>> fatal is not. If the log message is followed by an cold exit, ISTM that
>>> fatal is the right call, and I cannot help if other commands do not do that.
>>> ISTM more logical to align other commands to fatal when appropriate.
>
> I disagree.  pgbench is an outlier here.  There are 71 calls to
> pg_log_fatal() in src/bin/, and pgbench counts for 54 of them.  It
> would be more consistent to align pgbench with the others.

I do not understand your disagreement. Do you disagree about the expected 
semantics of fatal? Then why provide fatal if it should not be used?
What is the expected usage of fatal?

I do not dispute that pgbench is a statistical outlier. However, Pgbench 
is somehow special because it does not handle a lot of errors, hence a lot 
of "fatal & exit" pattern is used, and the user has to restart. There are 
76 calls to "exit" from pgbench, but only 23 from psql which is much 
larger. ISTM that most "normal" pg programs won't do that because they are 
nice and try to handle errors.

So for me "fatal" is the right choice before exiting with a non zero 
status, but if "error" is called instead I do not think it matters much, 
you do as you please.

>> I was surprised to discover a few weeks ago that pg_log_fatal() did not
>> terminate the program, which was my expectation.

Mine too.

-- 
Fabien.

Commits

  1. Add some missing exit() calls in error paths for various binaries

  2. Add missing exit() in pg_verifybackup when failing to find pg_waldump