Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

On 08/09/10 20:36, Markus Wanner wrote:
> On 09/06/2010 11:03 PM, Tom Lane wrote:
>> I don't entirely see the point of opening ourselves up to the risk of
>> using a pselect that's not safe under the hood.
>
> It should be possible to reliably determine the platforms that provide
> an atomic pselect(). For those, I'm hesitant to use a "trick", where
> pselect() clearly provides a simpler and more "official" alternative.
> Especially considering that those platforms form the vast majority for
> running Postgres on.

Perhaps, but I'm equally concerned that having different implementations 
for different platforms means that all implementations get less testing 
than if we use only one. Because of that I'm actually reluctant to even 
use poll() where available instead of select(). At least in the first 
phase, until someone demonstrates that there's a measurable difference 
in performance. We only call poll/select when we're about to sleep, so 
it's not really that performance critical anyway.

> What I'm most concerned about is the write() syscall within the signal
> handler. If that fails for another reason than those covered, we miss
> the signal. As Heikki points out in the comment, it's hard to deal with
> such a failure.

Yeah, there isn't much you can do about it. Perhaps you could set a 
"mayday flag" (a global boolean variable) if it fails, and check that in 
the main loop, elogging a warning there instead. But I don't think we 
need to go to such lengths, realistically the write() will never fail or 
you have bigger problems.

> Maybe you can read more than one byte at a time in drainSelfPipe(), to
> save some syscalls?

Perhaps, although it should be very rare to have more than one byte in 
the pipe. SetLatch doesn't write another byte if the latch is already 
set, so you only get multiple bytes in the pipe if many processes set 
the latch at the same instant.

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