On Wed, Nov 19, 2014 at 1:01 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Tue, Nov 18, 2014 at 5:14 PM, Dimitri Fontaine > <dimi...@2ndquadrant.fr> wrote: >> Robert Haas <robertmh...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers