From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: HS locking broken in HEAD |
Date: | 2013-01-18 15:15:20 |
Message-ID: | CA+Tgmob_SkqkQW1PxjgaGBSh7ha5-ww1wPV+0tPbn-aRAGs70Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jan 17, 2013 at 9:00 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > I think it might also be a dangerous assumption for shared objects?
>>
>> Locks on shared objects can't be taken via the fast path. In order to
>> take a fast-path lock, a backend must be bound to a database and the
>> locktag must be for a relation in that database.
>
> I assumed it wasn't allowed, but didn't find where that happens at first
> - I found it now though (c.f. SetLocktagRelationOid).
>
>> > A patch minimally addressing the is attached, but it only addresses part
>> > of the problem.
>>
>> Isn't the right fix to compare proc->databaseId to
>> locktag->locktag_field1 rather than to MyDatabaseId? The subsequent
>> loop assumes that if the relid matches, the lock tag is an exact
>> match, which will be correct with that rule but not the one you
>> propose.
>
> I don't know much about the locking code, you're probably right. I also
> didn't look very far after finding the guilty commit.
>
> (reading code)
>
> Yes, I think you're right, that seems to be the actually correct fix.
>
> I am a bit worried that there are more such assumptions in the code, but
> I didn't find any, but I really don't know that code.
I found two instances of this. See attached patch.
Can you test whether this fixes it for you?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment | Content-Type | Size |
---|---|---|
fix-mydatabaseid-compare.patch | application/octet-stream | 1.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2013-01-18 15:23:07 | Re: Re: [PATCH 1/5] Centralize Assert* macros into c.h so its common between backend/frontend |
Previous Message | Andres Freund | 2013-01-18 15:09:18 | Re: Re: [PATCH 1/5] Centralize Assert* macros into c.h so its common between backend/frontend |