Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe perprocess

Поиск
Список
Период
Сортировка
От Kyotaro HORIGUCHI
Тема Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe perprocess
Дата
Msg-id 20180906.185715.126482377.horiguchi.kyotaro@lab.ntt.co.jp
обсуждение исходный текст
Ответ на Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe per process  (Thomas Munro <thomas.munro@enterprisedb.com>)
Ответы Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe per process  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe per process  (Thomas Munro <thomas.munro@enterprisedb.com>)
Список pgsql-hackers
At Sun, 2 Sep 2018 07:04:19 +1200, Thomas Munro <thomas.munro@enterprisedb.com> wrote in
<CAEepm=0=PkSXQ5oNU8BhY1DEzxw_EspsU14D_zAPnj+fBAjxFQ@mail.gmail.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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alexander Korotkov
Дата:
Сообщение: Re: Bug in ginRedoRecompress that causes opaque data on page to be overrun
Следующее
От: Andrey Lepikhov
Дата:
Сообщение: Re: [WIP] [B-Tree] Retail IndexTuple deletion