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

At Sun, 2 Sep 2018 07:04:19 +1200, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote in <CAEepm=0=PkSXQ5oNU8BhY1DEzxw_EspsU14D_zAPnj+fBAjxFQ(at)mail(dot)gmail(dot)com>
> > > # Is it intentional that the patch doesn't touch pgstat.c?
> >
> > Yes. pgstat.c still uses WL_POSTMASTER_DEATH because it does
> > something special: it calls pgstat_write_statsfiles() before it exits.

Mmm. Exactly..

> Rebased.

Thank you for the new version.

===
In sysloger.c, cur_flags is (just set but) no longer used.

===
In latch.c,

- The parentheses around the symbols don't seem to be needed.
| (wakeEvents & (WL_EXIT_ON_PM_DEATH)) != 0 ||
| (wakeEvents & (WL_POSTMASTER_DEATH)) != 0);

- The following assertion looks contradicting to the comment.
| /* Postmaster-managed callers must handle postmaster death somehow. */
| Assert(!IsUnderPostmaster ||
| (wakeEvents & (WL_EXIT_ON_PM_DEATH)) != 0 ||
| (wakeEvents & (WL_POSTMASTER_DEATH)) != 0);

(Maybe Assert(IsUnderPost && (wakeEv & (WL_EXI | WL_PO)) != 0);)

And don't we need a description about this restriction in the
function comment?

- I think it may be better that the followings had parentheses
around '&' expressions.

| if (wakeEvents & WL_POSTMASTER_DEATH && IsUnderPostmaster)
| if (wakeEvents & WL_EXIT_ON_PM_DEATH && IsUnderPostmaster)

===
All the caller sites of WaitLatch, WaitLatchOrSocket and
WaitEventSetWait are covered by this patch and all them look
fine.

bgworker.c, pgstat.c, be-secure-openssl.c, be-secure.c:
Not modified on purpose. WL_EXIT_POSTMASTER_DEATH is not proper
to use there.

pgarch.c, syncrep.c, walsender.c:
Removed redundant check of postmaster death.

syslogger.c:
Left as it doesn't exit at postmaster death on purpose. Uses
reusable wait event set.

walrceiver.c:
Adds new bailing out point in WalRcvWaitForStartPosition and it
seems reasonable.

shm_mq.c:
Adds PMdie bail out in shm_mq_send/receive_bytes, wait_internal.
It looks fine.

postgres_fdw/connection.c:
Adds pm_die bailout while getting result. This seems to be a bug fix.
I'm fine with it included in this patch.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center