Thread: pgsql: Improve tests for postmaster death in auxiliary processes.

pgsql: Improve tests for postmaster death in auxiliary processes.

From
Tom Lane
Date:
Improve tests for postmaster death in auxiliary processes.

In checkpointer and walwriter, avoid calling PostmasterIsAlive unless
WaitLatch has reported WL_POSTMASTER_DEATH.  This saves a kernel call per
iteration of the process's outer loop, which is not all that much, but a
cycle shaved is a cycle earned.  I had already removed the unconditional
PostmasterIsAlive calls in bgwriter and pgstat in previous patches, but
forgot that WL_POSTMASTER_DEATH is supposed to be treated as untrustworthy
(per comment in unix_latch.c); so adjust those two cases to match.

There are a few other places where the same idea might be applied, but only
after substantial code rearrangement, so I didn't bother.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/fd71421b0187de0e2bf76ff66b4a9433bd96c4a0

Modified Files
--------------
src/backend/postmaster/bgwriter.c     |    6 ++++--
src/backend/postmaster/checkpointer.c |   23 +++++++++++++----------
src/backend/postmaster/pgstat.c       |    9 +++++++--
src/backend/postmaster/walwriter.c    |   23 +++++++++++++----------
4 files changed, 37 insertions(+), 24 deletions(-)


Re: pgsql: Improve tests for postmaster death in auxiliary processes.

From
Peter Geoghegan
Date:
On 10 May 2012 05:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> In checkpointer and walwriter, avoid calling PostmasterIsAlive unless
> WaitLatch has reported WL_POSTMASTER_DEATH.  This saves a kernel call per
> iteration of the process's outer loop, which is not all that much, but a
> cycle shaved is a cycle earned.  I had already removed the unconditional
> PostmasterIsAlive calls in bgwriter and pgstat in previous patches, but
> forgot that WL_POSTMASTER_DEATH is supposed to be treated as untrustworthy
> (per comment in unix_latch.c); so adjust those two cases to match.

I'm not sure why we're pushing the responsibility to call
PostmasterIsAlive() onto latch clients. Why not just do it within
WaitLatchOrSocket just as the WL_POSTMASTER_DEATH bit is set? It's not
as if someone could conceivably care that the Postmaster might have
died, but not enough to want to be sure.

--
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

Peter Geoghegan <peter@2ndquadrant.com> writes:
> On 10 May 2012 05:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I had already removed the unconditional
>> PostmasterIsAlive calls in bgwriter and pgstat in previous patches, but
>> forgot that WL_POSTMASTER_DEATH is supposed to be treated as untrustworthy
>> (per comment in unix_latch.c); so adjust those two cases to match.

> I'm not sure why we're pushing the responsibility to call
> PostmasterIsAlive() onto latch clients. Why not just do it within
> WaitLatchOrSocket just as the WL_POSTMASTER_DEATH bit is set? It's not
> as if someone could conceivably care that the Postmaster might have
> died, but not enough to want to be sure.

Hmm ... yeah, that would simplify the API if we didn't have to have the
caveat about the bits being untrustworthy.  The only objection I can see
is that this might represent a useless PostmasterIsAlive call for those
callers that aren't inspecting the result bit.  However, at this point
we've more or less decided that those callers aren't
performance-critical anyway; anyone who thinks they are should be
rearranging their code to remove the separate PostmasterIsAlive calls.

So, yeah, seems like a good idea.  Any objections out there?

            regards, tom lane