Re: LockDatabaseObject vs. LockSharedObject

On Sun, Aug 15, 2010 at 3:22 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> It seems suspicious to me that LockSharedObject() calls
>> AcceptInvalidationMessges() and LockDatabaseObject() does not.  Since
>> the only caller of LockSharedObject() at present is
>> AcquireDeletionLock(), I'm not sure there's an observable bug here at
>> the moment, but then again, I'm also not sure there isn't.
>
> ITYM the only caller of LockDatabaseObject is AcquireDeletionLock.

Right, sorry.

> Given that the other logic path in AcquireDeletionLock calls
> LockRelationOid, which *will* result in an AcceptInvalidationMessages
> call, it does seem pretty suspicious.  The type of bug that you'd
> expect to have from this is that a recent DDL change on a non-relation
> object might not be seen by a concurrent drop being done on that object.

That's what I was thinking, too.

> I'm not sure that we have any non-relation objects that are both complex
> enough and changeable enough for there to be an observable bug here,
> but it seems like a risk factor going forward.  It seems to me both safe
> and reasonable to add an AcceptInvalidationMessages call in HEAD.

My "comment refactoring" patch (already posted to the list) makes this
change among others. With the added locking introduce by that patch,
it's possible to demonstrate a live bug here without
AcceptInvalidationMessages(). I'd sort of like to get that committed,
by the way, though I haven't had time to revise it yet per the
feedback already received. To get anything useful done for
SE-PostgreSQL this release is going to require AT LEAST one more
good-sized patch after that one, and I'd like to not be landing it at
the very end of the release cycle when there's no time for second or
third thoughts.

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