On 8 July 2011 15:58, Florian Pflug <fgp@phlo.org> wrote:
> SyncRepWaitForLSN() says
> /*
> * Wait on latch for up to 60 seconds. This allows us to check for
> * postmaster death regularly while waiting. Note that timeout here
> * does not necessarily release from loop.
> */
> WaitLatch(&MyProc->waitLatch, 60000000L);
>
> I guess that 60-second timeout is unnecessary now that we'll wake up
> on postmaster death anyway.
We won't wake up on Postmaster death here, because we haven't asked to
- not yet, anyway. We're just using the new interface here for that
one function call in v8.
> Also, none of the callers of WaitLatch() seems to actually inspect
> the WL_POSTMASTER_DEATH bit of the result. We might want to make
> their !PostmasterIsAlive() check conditional on the WL_POSTMASTER_DEATH
> bit being set. At least in the case of SyncRepWaitForLSN(), it seems
> that avoiding the extra read() syscall might be beneficial.
I don't think so. Postmaster death is an anomaly, so why bother with
any sort of optimisation for that case? Also, that's exactly the sort
of thing that we're trying to caution callers against doing with this
comment:
"That should be rare in practice, but the caller should not use the
return value for anything critical, re-checking the situation with
PostmasterIsAlive() or read() on a socket if necessary."
You might say that the only reason we even bother reporting postmaster
death in the returned bitfield is because there is an expectation that
it will report it, given that we use the same masks on wakeEvents to
inform the function what events we'll actually be waiting on for the
call.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services