Re: Calling pgstat_report_wait_end() before ereport(ERROR)

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