Re: Advisory locks seem rather broken

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Subject: Re: Advisory locks seem rather broken
Date: 2012-05-30 20:26:09
Message-ID: CA+TgmoYXoHHc4PPoSOWmF_xgfBGsZPy_Uj0j2Oh-ZhLtHAwSkQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 4, 2012 at 9:17 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, May 3, 2012 at 5:18 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

This issue had slipped my mind, but Erik's report about another
fast-path locking problem jogged my memory, so I repaired this while I
was at it.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message james 2012-05-30 20:28:43 Re: Fake async rep target
Previous Message Robert Haas 2012-05-30 20:25:03 Re: FailedAssertion("!(PrivateRefCount[i] == 0)", File: "bufmgr.c", Line: 1741