From: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
---|---|
To: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] PATCH: Keep one postmaster monitoring pipe per process |
Date: | 2018-10-23 04:26:37 |
Message-ID: | CAEepm=3BxOHw63Anwyx=pyy7Ju8cW6T0ZXNeKQS=WZp80hv_rg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 6, 2018 at 9:57 PM Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> In sysloger.c, cur_flags is (just set but) no longer used.
Right. Fixed.
> ===
> 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);
Fixed.
> And don't we need a description about this restriction in the
> function comment?
Ok, added.
> - 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)
Fixed. Also I was a little inconsistent about whether to use implicit
or explicit comparison with 0, and decided to go for the former.
> All the caller sites of WaitLatch, WaitLatchOrSocket and
> WaitEventSetWait are covered by this patch and all them look
> fine.
Thanks for the review!
While rebasing, I also changed WL_EXIT_ON_PM_DEATH's means of exit
from exit(1) to proc_exit(1). In master we're quite inconsistent
about that as you can see from the lines removed by this patch, but
the comments for proc_exit() seem to insist that it is the one true
way out. Other than an elog(DEBUG3) message, the main difference
between proc_exit() and direct exit() seems to be the PROFILE_PID_DIR
stuff that changes directory on the way out the door for gprof users.
It looks like per-pid gmon files could have been achieved by setting
environment variables[1][2], but I guess there is some value in doing
it the same way on every platform.
[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=gmon/gmon.c;hb=HEAD#l354
[2] https://github.com/freebsd/freebsd/blob/master/lib/libc/gmon/gmon.c#L154
--
Thomas Munro
http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
0001-Add-WL_EXIT_ON_PM_DEATH-pseudo-event-v4.patch | application/octet-stream | 38.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2018-10-23 04:37:01 | Re: Synchronous replay take III |
Previous Message | Kyotaro HORIGUCHI | 2018-10-23 04:19:29 | Re: Speeding up text_position_next with multibyte encodings |