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