On Wed, Oct 22, 2014 at 3:06 AM, Andreas Karlsson <andr...@proxel.se> wrote: > I have been thinking about why we need to grab an AccessExclusiveLock on the > table with the PK when we add a foreign key. Adding new tables with foreign > keys to old ones is common so it would be really nice if the lock strength > could be reduced. > > A comment in the code claims that we need to lock so no rows are deleted > under us and that adding a trigger will lock in AccessExclusive anyway. But > with MVCC catalog access and the removal of SnapshotNow I do not see why > adding a trigger would require an exclusive lock. Just locking for data > changes should be enough.
The use of MVCC catalog access doesn't necessarily mean that adding a trigger doesn't require an AccessExclusive lock. Those changes - if I dare to say so myself - solved a complex and important problem, but only one of many problems in this area, and people seem prone to thinking that they solved more problems than they in fact did. I think instead of focusing on foreign keys, we should rewind a bit and think about the locking level required to add a trigger. If we figure something out there, then we can consider how it affects foreign keys. I went looking for previous discussions of remaining hazards and found these postings: http://www.postgresql.org/message-id/ca+tgmoy4glsxzk0tao29-ljtcuj0sl1xwcwq51xb-hfysgi...@mail.gmail.com http://www.postgresql.org/message-id/20893.1393892...@sss.pgh.pa.us http://www.postgresql.org/message-id/20140306224340.ga3551...@tornado.leadboat.com As far as triggers are concerned, the issue of skew between the transaction snapshot and what the ruleutils.c snapshots do seems to be the principal issue. Commit e5550d5fec66aa74caad1f79b79826ec64898688 changed pg_get_constraintdef() to use an MVCC snapshot rather than a current MVCC snapshot; if that change is safe, I am not aware of any reason why we couldn't change pg_get_triggerdef() similarly. Barring further hazards I haven't thought of, I would expect that we could add a trigger to a relation with only ShareRowExclusiveLock. Anything less than ShareRowExclusiveLock would open up strange timing races around the firing of triggers by transactions writing the table: they might or might not notice that a trigger had been added before end-of-transaction, depending on the timing of cache flushes, which certainly seems no good. But even RowExclusiveLock seems like a large improvement over AccessExclusiveLock. When a constraint trigger - which is used to implement a foreign key - is added, there are actually TWO tables involved: the table upon which the trigger will actually fire, and some other table which is mentioned in passing in the trigger definition. It's possible that the locking requirements for the secondary table are weaker since I don't think the presence of the trigger actually affects runtime behavior there. However, there's probably little point in try to weaken the lock to less than the level required for the main table because a foreign key involves adding referential integrity triggers to both tables. So I tentatively propose (and with due regard for the possibility others may see dangers that I've missed) that a reasonable goal would be to lower the lock strength required for both CREATE TRIGGER and ADD FOREIGN KEY from AccessExclusiveLock to ShareRowExclusiveLock, allowing concurrent SELECT and SELECT FOR SHARE against the tables, but not any actual write operations. -- 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
