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

From Tom Lane
Subject Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
Date
Msg-id 22650.1283525497@sss.pgh.pa.us
Whole thread Raw
In response to Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)  (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>)
Responses Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)  (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>)
List pgsql-hackers
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> On 02/09/10 23:13, Tom Lane wrote:
>>> (Yeah, I realize the caller
>>> could look into the latch to find that out, but callers really ought to
>>> treat latches as opaque structs.)

> Hmm, maybe we need a TestLatch function to check if a latch is set.

+1.  It could be a macro for now.  I wish that we could declare Latch as
an opaque struct, but we probably need to let callers embed it in a
larger struct, so we'll just have to rely on callers to code cleanly.

> I also realized that the timeout handling is a bit surprising with 
> interrupts. After EINTR we call select() again with the same timeout, so 
> a signal effectively restarts the timer.

Actually it's platform-dependent.  On some machines (I think
BSD-derived) the value of the timeout struct gets reduced by the elapsed
time before returning, so that if you just loop around you don't get
more than the originally specified delay time in total.

> We seem to have similar 
> behavior in a couple of other places, in pgstat.c and auth.c. So maybe 
> that's OK and just needs to be documented, but I thought I'd bring it up.

Yeah.  I am hoping that this facility will greatly reduce the need for
callers to care about the exact timeout delay, so I think that what we
should do for now is just document that the timeout might be several
times as long as specified, and see how it goes from there.

We could fix the problem if we had to, by adding some gettimeofday()
calls and computing the remaining delay time each time through the
loop.  But let's avoid doing that until it's clear we need it.

>> Also, using sig_atomic_t for owner_pid is entirely not sane.

> Hmm, true, it doesn't need to be set from signal handler, but is there 
> an atomicity problem if one process calls ReleaseLatch while another 
> process is in SetLatch?

If there is *any* possibility of that happening then you have far worse
problems than whether the field is atomically readable or not: the
behavior will be unpredictable at just slightly larger timescales.
This is the reason why I think it'd be better if ReleaseLatch simply
didn't exist.  That'd discourage people from designing dynamic latch
structures, which are fundamentally going to be subject to race
conditions.
        regards, tom lane


pgsql-hackers by date:

Previous
From: PostgreSQL - Hans-Jürgen Schönig
Date:
Subject: Re: plan time of MASSIVE partitioning ...
Next
From: Tom Lane
Date:
Subject: Re: Streaming a base backup from master