On Wed, Nov 19, 2014 at 1:01 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Nov 18, 2014 at 5:14 PM, Dimitri Fontaine
> <dimitri@2ndquadrant.fr> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> It seems pretty weird, also, that the event trigger will fire after
>>> we've taken AccessExclusiveLock when you cluster a particular
>>> relation, and before we've taken AccessExclusiveLock when you cluster
>>> database-wide. That's more or less an implementation artifact of the
>>> current code that we're exposing to the use for, really, no good
>>> reason.
>>
>> In the CLUSTER implementation we have only one call site for invoking
>> the Event Trigger, in cluster_rel(). While it's true that in the single
>> relation case, the relation is opened in cluster() then cluster_rel() is
>> called, the opening is done with NoLock in cluster():
>>
>> rel = heap_open(tableOid, NoLock);
>>
>> My understanding is that the relation locking only happens in
>> cluster_rel() at this line:
>>
>> OldHeap = try_relation_open(tableOid, AccessExclusiveLock);
>>
>> Please help me through the cluster locking strategy here, I feel like
>> I'm missing something obvious, as my conclusion from re-reading the code
>> in lights of your comment is that your comment is not accurate with
>> respect to the current state of the code.
>
> Unless I'm missing something, when you cluster a particular relation,
> cluster() does this:
>
> /* Find and lock the table */
> rel = heap_openrv(stmt->relation, AccessExclusiveLock);
>
> I don't see the "rel = heap_open(tableOid, NoLock);" line you quoted anywhere.
...which is because I have the 9.1 branch checked out. Genius. But
what I said originally is still true, because the current code looks
like this:
/* Find, lock, and check permissions on the table */ tableOid =
RangeVarGetRelidExtended(stmt->relation,
AccessExclusiveLock,
false, false,
RangeVarCallbackOwnsTable, NULL); rel = heap_open(tableOid, NoLock);
It's true that the heap_open() is not acquiring any lock. But the
RangeVarGetRelidExtended() call right before it is.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
