Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe per process

Hello.

At Wed, 18 Jul 2018 14:02:47 +1200, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote in <CAEepm=3t57KBxb90CTqnDZiSTnTq3jwxZUc0zaFDiaOkGQqNxA(at)mail(dot)gmail(dot)com>
> On Sat, Jul 14, 2018 at 5:34 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> > On 18/04/18 09:55, Thomas Munro wrote:
> >> Here's a draft patch that does that. One contentious question is:
> >> should you have to opt *in* to auto-exit-on-postmaster death? Andres
> >> opined that you should. I actually think it's not so bad if you don't
> >> have to do that, and instead have to opt out. I think of it as a kind
> >> of 'process cancellation point' or a quiet PANIC that you can opt out
> >> of. It's nice to remove the old boilerplate code without having to
> >> add a new boilerplate event that you have to remember every time. Any
> >> other opinions?
> >
> > Hmm. Exiting on postmaster death by default does feel a bit too magical to
> > me. But as your patch points out, there are currently no places where you
> > *don't* want to exit on postmaster death, some callers just prefer to handle
> > it themselves. And I like having a default or something to make sure that
> > all call sites in the future will also exit quickly.
> >
> > I'd suggest that we add a new WL_EXIT_ON_POSTMASTER_DEATH flag, making it
> > opt-on.
>
> Ok, it's now 2 against 1. So here's a version that uses an explicit
> WL_EXIT_ON_PM_DEATH value. I like that name because it's shorter and
> more visually distinctive (harder to confuse with
> WL_POSTMASTER_DEATH).
>
> > But add an assertion in WaitLatchOrSocket:
> >
> > Assert ((wakeEvents & (WL_EXIT_POSTMASTER_DEATH | WL_POSTMASTER_DEATH)) !=
> > 0);
>
> Ok. Done for the WaitLatchXXX() interface. The WaitEventSet
> interface (rarely used directly) is less amenable to that change.
>
> Here are some of the places I had to add WL_EXIT_ON_PM_DEATH:
> gather_readnext(), shm_mq_send_bytes(), shm_mq_receive_bytes(),
> shm_mq_wait_internal(), ProcSleep(), ProcWaitForSignal(), pg_sleep(),
> pgfdw_get_result().
>
> Was it intentional that any of those places don't currently exit on
> postmaster vaporisation?

I think that backends are supposed to complete running query even
if postmaster dies meanwhile and currently that seems
true. pgfdw_get_result seems to be following the policy. Perhaps
it's the same for all of the functions listed above.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center