Dimitri Fontaine wrote: > 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.
Almost the whole of that function is conditions to bail out clustering the relation if things have changed since the relation list was collected. It seems wrong to invoke the event trigger in all those cases; it's going to fire spuriously. I think you should move the invocation of the event trigger at the end, just before rebuild_relation is called. Not sure where relative to the predicate lock stuff therein; probably before, so that we avoid doing that dance if the event trigger function decides to jump ship. In ATRewriteTables, it seems wrong to call it after make_new_heap. If the event trigger function aborts, we end up with useless work done there; so I think it should be called before that. Also, why do you have the evt_table_rewrite_fired stuff? I think you should fire one event per table, no? The second ATRewriteTable call in ATRewriteTables does not actually rewrite the table; it only scans it to verify constraints. So I'm thinking you shouldn't call this event trigger there. Or, if we decide we want this, we probably also need something for the table scans in ALTER DOMAIN too. You still have the ANALYZE thing in docs, which now should be removed. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
