Re: Strange locking choices in pg_shdepend.c - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Strange locking choices in pg_shdepend.c
Date
Msg-id 20080121223127.GE12482@alvh.no-ip.org
Whole thread Raw
In response to Strange locking choices in pg_shdepend.c  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Strange locking choices in pg_shdepend.c  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Strange locking choices in pg_shdepend.c  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
Tom Lane wrote:
> I came across some rather strange choices of lock levels in pg_shdepend.c.
> 
> Why does shdepDropOwned() take AccessExclusiveLock on pg_shdepend?
> Seems like RowExclusiveLock should be sufficient.  If it isn't
> sufficient, I wonder whether the other functions in here are taking
> strong enough locks.

Hmm, I can't recall nor deduce any reason for that.  Perhaps the
intention was to protect against itself; but I think this should only
matter if we're dropping the same role concurrently (otherwise the
to-be-dropped objects would be disjoint sets, so it doesn't matter),
which should be already protected by the lock on the role itself.

Hmm, unless revoking privileges concurrently, for two different users on
the same object could cause a problem?  I don't see us grabbing a lock
on the object itself -- does this matter?


> It's probably not a good idea to have shdepReassignOwned() take only
> AccessShareLock on pg_shdepend.  Even though the function itself
> merely reads the table, it is going to call functions that will take
> RowExclusiveLock, meaning that we're setting ourselves up for potential
> deadlock failures due to lock-upgrade.  It'd be safer (and faster too)
> to just hold RowExclusiveLock through the whole operation.

Huh, correct.  I think this was just an oversight.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: [GENERAL] setof record "out" syntax and returning records
Next
From: Tom Lane
Date:
Subject: Thoughts about bug #3883