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

From Jeff Davis
Subject Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
Date
Msg-id 1284308115.18130.28.camel@jdavis-laptop
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!)
List pgsql-hackers
On Sat, 2010-09-11 at 19:15 +0300, Heikki Linnakangas wrote:
> Committed. I'll take a look at making walreceiver respond quickly when 
> WAL arrives in the standby, using latches, but that shouldn't interfere 
> with what you're doing.

I glanced at the code, and I see (in OwnLatch()):

+       if (latch->owner_pid != 0)
+               elog(ERROR, "latch already owned");
+       latch->owner_pid = MyProcPid;

But it looks like there may be a race there. I assume the callers are
supposed to have a lock at this point (and it looks like the current
caller is safe, but I didn't look in detail). Something still seems
strange to me though -- why throw an ERROR there if it can't happen (or
indicates an inconsistent state when it does happen)? And it doesn't
look safe to _not_ throw an error (due to a race) if it does happen.

It seems like OwnLatch is only supposed to be used when you_not_ to
throw an error in the event of a race (I don't think it is). already
know that you own it, because there's no error code returned or blocking
behavior, it just throws an ERROR. But if that's the case, what's the
point of OwnLatch()?

Perhaps add a few comments to describe to other users of the API how to
properly own a shared latch?

Regards,Jeff Davis



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: cvs2git reports a "sprout" from a nonexistent commit?
Next
From: Tom Lane
Date:
Subject: Re: Interruptible sleeps (was Re: CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)