On Tue, Apr 16, 2019 at 2:45 PM Michael Paquier <mich...@paquier.xyz> wrote: > > 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?
Agreed. There are also some code which raise an ERROR after close a transient file but I think it's a good idea to not include them for safety. It looks to me that the patch you proposed cleans places as much as we can do. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
