Re: Calling pgstat_report_wait_end() before ereport(ERROR) - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Calling pgstat_report_wait_end() before ereport(ERROR)
Date
Msg-id 20190416054443.GF2673@paquier.xyz
Whole thread Raw
In response to Re: Calling pgstat_report_wait_end() before ereport(ERROR)  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: Calling pgstat_report_wait_end() before ereport(ERROR)
Re: Calling pgstat_report_wait_end() before ereport(ERROR)
List pgsql-hackers
On Fri, Apr 12, 2019 at 10:06:41PM +0900, Masahiko Sawada wrote:
> But I think that's not right, I've checked the code. If the startup
> process failed in that function it raises a FATAL and recovery fails,
> and if checkpointer process does then it calls
> pgstat_report_wait_end() in CheckpointerMain().

Well, the point is that the code raises an ERROR, then a FATAL because
it gets upgraded by recovery.  The take, at least it seems to me, is
that if any new caller of the function misses to clean up the event
then the routine gets cleared.  So it seems to me that the current
coding is aimed to be more defensive than anything.  I agree that
there is perhaps little point in doing so.  In my experience a backend
switches very quickly back to ClientRead, cleaning up the previous
event.  Looking around, we have also some code paths in slot.c and
origin.c which close a transient file, clear the event flag...  And
then PANIC, which makes even less sense.

In short, I tend to think that the attached is an acceptable cleanup.
Thoughts?
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: [Patch] Mingw: Fix import library extension, build actual staticlibraries
Next
From: Ashwin Agrawal
Date:
Subject: Re: Zedstore - compressed in-core columnar storage