From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Frontend error logging style |
Date: | 2022-02-23 03:11:37 |
Message-ID: | 20220223031137.grgocasyh7qs2tn3@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2022-02-22 18:23:19 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Fri, Nov 19, 2021 at 5:17 AM Peter Eisentraut
> > <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
> >> If people want to do that kind of thing (I'm undecided whether the
> >> complexity is worth it), then make it a different API. The pg_log_*
> >> calls are for writing formatted output. They normalized existing
> >> hand-coded patterns at the time. We can wrap another API on top of them
> >> that does flow control and output. The pg_log_* stuff is more on the
> >> level of syslog(), which also just outputs stuff. Nobody is suggesting
> >> that syslog(LOG_EMERG) should exit the program automatically. But you
> >> can wrap higher-level APIs such as ereport() on top of that that might
> >> do that.
That'd maybe be a convincing argument in a project that didn't have
elog(FATAL).
> > Yeah, that might be a way forward.
>
> This conversation seems to have tailed off without full resolution,
> but I observe that pretty much everyone except Peter is on board
> with defining pg_log_fatal as pg_log_error + exit(1). So I think
> we should just do that, unless Peter wants to produce a finished
> alternative proposal.
What about adding a pg_fatal() that's pg_log_fatal() + exit()? That keeps
pg_log_* stuff "log only", but adds something adjacent enough to hopefully
reduce future misunderstandings?
To further decrease the chance of such mistakes, we could rename
pg_log_fatal() to something else. Or add a a pg_log_fatal() function that's
marked pg_nodiscard(), so that each callsite explicitly has to be (void)
pg_log_fatal().
> I've now gone through and fleshed out the patch I sketched upthread.
> This exercise convinced me that we absolutely should do something
> very like this, because
>
> (1) As it stands, this patch removes a net of just about 1000 lines
> of code. So we clearly need *something*.
> (2) The savings would be even more, except that people have invented
> macros for pg_log_error + exit(1) in at least four places already.
> (None of them quite the same of course.) Here I've just fixed those
> macro definitions to use pg_log_fatal. In the name of consistency, we
> should probably get rid of those macros in favor of using pg_log_fatal
> directly; but I've not done so here, since this patch is eyewateringly
> long and boring already.
Agreed.
I don't care that much about the naming here.
For a moment I was wondering whether there'd be a benefit for having the "pure
logging" stuff be in a different static library than the erroring variant. So
that e.g. libpq's check for exit() doesn't run into trouble. But then I
remembered that libpq doesn't link against common...
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-02-23 03:44:25 | Re: Frontend error logging style |
Previous Message | Peter Smith | 2022-02-23 03:07:45 | Re: Design of pg_stat_subscription_workers vs pgstats |