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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
Date: 2010-09-03 14:51:37
Message-ID: 22650.1283525497@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2010-09-03 15:01:36 Re: Streaming a base backup from master
Previous Message PostgreSQL - Hans-Jürgen Schönig 2010-09-03 14:46:26 Re: plan time of MASSIVE partitioning ...