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

On 06/09/10 19:27, Heikki Linnakangas wrote:
> On 06/09/10 17:18, Tom Lane wrote:
>> BTW, on reflection the AcquireLatch/ReleaseLatch terminology seems a bit
>> ill chosen: ReleaseLatch sounds way too much like something that would
>> just unlock or clear the latch. Perhaps OwnLatch/DisownLatch, or
>> something along that line.
>
> Yeah, I see what you mean. Although, maybe it's just me but Own/Disown
> looks ugly. Anyone have a better suggestion?

Magnus suggested AssociateLatch, given that the description of the
function is that it associates the latch with the current process. I
went with Own/Disown after all, it feels more precise, and having made
the changes it doesn't look that ugly to me anymore.

> Here's an updated patch, with all the issues reported this far fixed,
> except for that naming issue, and Fujii's suggestion to use poll()
> instead of select() where available. I've also polished it quite a bit,
> improving comments etc. Magnus, can you take a look at the Windows
> implementation to check that it's sane? At least it seems to work.

We discussed the patch over IM, there's one point minor point I'd like
to get into the archives:

> It seems that NumSharedLatches() is entirely wrongly placed if it's in
> the win32 specific code! That needs to be somewhere shared, so people find it,

Yeah. There's a notice of that in OwnLatch(), but it would be nice if we
could make it even more prominent. One idea is to put in latch.h as:

#define NumSharedLatches() (max_wal_senders /* + something else in the
future */ )

When it's a #define, we don't need to put #include "walsender.h" in
latch.h, it's enough to put it in win32_latch.c. It's a bit weird to
have a #define in one header file that doesn't work unless you #include
another header file in where you use it, but it would work. Any opinions
on whether that's better than having NumSharedLatches() defined in the
Win32-specific win32_latch.c file? I'm inclined to leave it as it is, in
win32_latch.c, but I'm not sure.

Barring any last-minute objections, I'll commit this in the next few
days. This patch doesn't affect walreceiver yet; I think the next step
is to use the latches to eliminate the polling loop in walreceiver too,
so that as soon as a piece of WAL is fsync'd to disk in the standby,
it's applied.

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