Re: Advisory locks seem rather broken - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Advisory locks seem rather broken
Date
Msg-id CA+TgmoZwNupiyGLjA6HFp-QWqaM8Cx4-5r6YJjU961Erg895mA@mail.gmail.com
Whole thread Raw
In response to Re: Advisory locks seem rather broken  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Advisory locks seem rather broken  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Advisory locks seem rather broken  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Thu, May 3, 2012 at 5:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ... btw, it appears to me that the "fast path" patch has broken things
> rather badly in LockReleaseAll.  AFAICS it's not honoring either the
> lockmethodid restriction nor the allLocks restriction with respect to
> fastpath locks.  Perhaps user locks and session locks are never taken
> fast path, but still it would be better to be making those checks
> further up, no?

User locks are never taken fast path, but session locks can be, so I
think you're right that there is a bug here.  I think what we should
probably do is put the nLocks == 0 test before the lockmethodid and
allLocks checks, and then the fast path stuff after those two checks.

In 9.1, we just did this:
               if (locallock->proclock == NULL || locallock->lock == NULL)               {                       /*
                  * We must've run out of shared memory while 
trying to set up this                        * lock.  Just forget the local entry.                        */
          Assert(locallock->nLocks == 0);                       RemoveLocalLock(locallock);
continue;              } 

...and I just shoved the new logic into that stanza without thinking
hard enough about what order to do things in.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: Future In-Core Replication
Next
From: Tom Lane
Date:
Subject: Re: Advisory locks seem rather broken