Thanks for giving this your attention Fujii. Attached patch addresses
your concerns.
On 20 June 2011 05:53, Fujii Masao <masao.fujii@gmail.com> wrote:
> 'hifd' should be initialized to 'selfpipe_readfd' before the above
> 'if' block. Otherwise,
> 'hifd = postmaster_alive_fds[POSTMASTER_FD_WATCH]' might have no effect.
That's an oversight that I should have caught. Fixed.
> Why does the archive still need to wake up periodically?
That is consistent with its earlier behaviour..."she wakes up
occasionally to allow herself to be proactive". This comment does not
refer to the frequent updates that currently occur within the tight
polling loop. I think any concern about that would apply equally to
the original, unpatched code.
> Is the variable 'flag' really required? It's not used by fcntl() to
> set the fd nonblocking.
Yes, it's superfluous. Removed.
> Is FNONBLOCK equal to O_NONBLOCK? If yes, we should use O_NONBLOCK
> for the sake of consistency? In other code (e.g., noblock.c), O_NONBLOCK is used
> rather than FNONBLOCK.
FNONBLOCK is just an alias for O_NONBLOCK, so it seems reasonable to
be consistent in which variant we use. I have found suggestions that
it might break the build on OSX, so if that's true there's an
excellent reason to prefer the latter.
> I think that it's worth that walsender checks the postmaster death event. No?
It does check it, but only in the same way that it always has (a tight
polling loop). I would like to make walsender use the new
functionality. That is another patch though, that I thought best to
have independently reviewed, only when this patch is committed. I've
only made the walsender use the new interface, changing as little as
possible and not affecting walsender's behaviour, as a stopgap towards
that patch.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services