pg_log_fatal vs pg_log_error

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

pg_log_fatal vs pg_log_error

Antonin Houska-2
Can anyone please give me a hint (and possibly add some comments to the code)
when pg_log_fatal() should be used in frontend code and when it's appropriate
to call pg_log_error()? The current use does not seem very consistent.

I'd expect that the pg_log_fatal() should be called when the error is serious
enough to cause premature exit, but I can see cases where even pg_log_error()
is followed by exit(1). pg_waldump makes me feel that pg_log_error() is used
to handle incorrect user input (before the actual execution started) while
pg_log_fatal() handles error conditions that user does not fully control
(things that happen during the actual execution). But this is rather a guess.

--
Antonin Houska
Web: https://www.cybertec-postgresql.com


Reply | Threaded
Open this post in threaded view
|

Re: pg_log_fatal vs pg_log_error

Michael Paquier-2
On Mon, Jun 17, 2019 at 02:19:30PM +0200, Antonin Houska wrote:
> I'd expect that the pg_log_fatal() should be called when the error is serious
> enough to cause premature exit, but I can see cases where even pg_log_error()
> is followed by exit(1). pg_waldump makes me feel that pg_log_error() is used
> to handle incorrect user input (before the actual execution started) while
> pg_log_fatal() handles error conditions that user does not fully control
> (things that happen during the actual execution). But this is rather a guess.

I agree with what you say when pg_log_fatal should be used for an
error bad enough that the binary should exit immediately.  In the case
of pg_waldump, not using pg_log_fatal() makes the code more readable
because there is no need to repeat the "Try --help for more
information on a bad argument".  Have you spotted other areas of the
code where it makes sense to change a pg_log_error() + exit to a
single pg_log_fatal()?
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_log_fatal vs pg_log_error

Antonin Houska-2
Michael Paquier <[hidden email]> wrote:

> On Mon, Jun 17, 2019 at 02:19:30PM +0200, Antonin Houska wrote:
> > I'd expect that the pg_log_fatal() should be called when the error is serious
> > enough to cause premature exit, but I can see cases where even pg_log_error()
> > is followed by exit(1). pg_waldump makes me feel that pg_log_error() is used
> > to handle incorrect user input (before the actual execution started) while
> > pg_log_fatal() handles error conditions that user does not fully control
> > (things that happen during the actual execution). But this is rather a guess.
>
> I agree with what you say when pg_log_fatal should be used for an
> error bad enough that the binary should exit immediately.  In the case
> of pg_waldump, not using pg_log_fatal() makes the code more readable
> because there is no need to repeat the "Try --help for more
> information on a bad argument".

I'd understand this if pg_log_fatal() called exit() itself, but it does not
(unless I miss something).

> Have you spotted other areas of the code where it makes sense to change a
> pg_log_error() + exit to a single pg_log_fatal()?

I haven't done an exhaustive search so far, but as I mentioned above,
pg_log_fatal() does not seem to be "pg_log_error() + exit()".

--
Antonin Houska
Web: https://www.cybertec-postgresql.com


Reply | Threaded
Open this post in threaded view
|

Re: pg_log_fatal vs pg_log_error

Michael Paquier-2
On Mon, Jun 17, 2019 at 03:39:49PM +0200, Antonin Houska wrote:
> I'd understand this if pg_log_fatal() called exit() itself, but it does not
> (unless I miss something).

Oops.  My apologies.  I have my own wrapper of pg_log_fatal() for an
internal tool which does an exit on top of the logging in this case.
You are right the PG code does not exit() in this case.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_log_fatal vs pg_log_error

Peter Eisentraut-6
In reply to this post by Antonin Houska-2
On 2019-06-17 14:19, Antonin Houska wrote:
> Can anyone please give me a hint (and possibly add some comments to the code)
> when pg_log_fatal() should be used in frontend code and when it's appropriate
> to call pg_log_error()? The current use does not seem very consistent.

For a program that runs in a loop, like for example psql or
pg_receivewal, use error if the program keeps running and fatal if not.
For one-shot programs like for example createdb, there is no difference,
so we have used error in those cases.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: pg_log_fatal vs pg_log_error

Antonin Houska-2
Peter Eisentraut <[hidden email]> wrote:

> On 2019-06-17 14:19, Antonin Houska wrote:
> > Can anyone please give me a hint (and possibly add some comments to the code)
> > when pg_log_fatal() should be used in frontend code and when it's appropriate
> > to call pg_log_error()? The current use does not seem very consistent.
>
> For a program that runs in a loop, like for example psql or
> pg_receivewal, use error if the program keeps running and fatal if not.
> For one-shot programs like for example createdb, there is no difference,
> so we have used error in those cases.

That makes sense, but shouldn't then pg_log_fatal() perform exit(EXIT_FAILURE)
internally? Just like elog(FATAL) does on backend side.

Actually there are indications that someone would appreciate such behaviour
even in frontends.

In pg_rewind.h I see:

/* logging support */
#define pg_fatal(...) do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)

or this in pg_upgrade/util.c:

void
pg_fatal(const char *fmt,...)
{
        va_list args;

        va_start(args, fmt);
        pg_log_v(PG_FATAL, fmt, args);
        va_end(args);
        printf(_("Failure, exiting\n"));
        exit(1);
}

--
Antonin Houska
Web: https://www.cybertec-postgresql.com