Re: Latch implementation that wakes on postmaster death on both win32 and Unix

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>

From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
To: Peter Geoghegan <peter@2ndquadrant.com>
Cc: Fujii Masao <masao.fujii@gmail.com>, PG Hackers <pgsql-hackers@postgresql.org>, Florian Pflug <fgp@phlo.org>
Date: 2011-07-04T15:53:43Z
Lists: pgsql-hackers

Attachments

Ok, here's a new patch, addressing the issues Fujii raised, and with a 
bunch of stylistic changes of my own. Also, I committed a patch to 
remove silent_mode, so the fork_process() changes are now gone. I'm 
going to sleep over this and review once again tomorrow, and commit if 
it still looks good to me and no-one else reports new issues.

There's two small issues left:

I don't like the names POSTMASTER_FD_WATCH and POSTMASTER_FD_OWN. At a 
quick glance, it's not at all clear which is which. I couldn't come up 
with better names, so for now I just added some comments to clarify 
that. I would find WRITE/READ more clear, but to make sense of that you 
need to how the pipe is used. Any suggestions or opinions on that?

The BUGS section of Linux man page for select(2) says:

>        Under Linux, select() may report a socket file descriptor as "ready for
>        reading",  while nevertheless a subsequent read blocks.  This could for
>        example happen when data has arrived but  upon  examination  has  wrong
>        checksum and is discarded.  There may be other circumstances in which a
>        file descriptor is spuriously reported as ready.  Thus it may be  safer
>        to use O_NONBLOCK on sockets that should not block.

So in theory, on Linux you might WaitLatch might sometimes incorrectly 
return WL_POSTMASTER_DEATH. None of the callers check for 
WL_POSTMASTER_DEATH return code, they call PostmasterIsAlive() before 
assuming the postmaster has died, so that won't affect correctness at 
the moment. I doubt that scenario can even happen in our case, select() 
on a pipe that is never written to. But maybe we should add add an 
assertion to WaitLatch to assert that if select() reports that the 
postmaster pipe has been closed, PostmasterIsAlive() also returns false.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com