Re: [HACKERS] Reducing lock strength of adding foreign keys
On 12/17/2014 03:41 PM, Simon Riggs wrote: All of this is just a replay of the earlier conversations about reducing lock levels. My original patch did reduce lock levels for CREATE TRIGGER, but we stepped back from doing that in 9.4 until we had feedback as to whether the whole idea of reducing lock levels was safe, which Robert, Tom and others were slightly uncertain about. Is there anything different here from work in my original patch series? As far as I know, no. -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
On Wed, Dec 17, 2014 at 02:41:39PM +, Simon Riggs wrote: Is there anything different here from work in my original patch series? Not to my knowledge. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
On 25 October 2014 19:00, Noah Misch n...@leadboat.com wrote: 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. +1 All of this is just a replay of the earlier conversations about reducing lock levels. My original patch did reduce lock levels for CREATE TRIGGER, but we stepped back from doing that in 9.4 until we had feedback as to whether the whole idea of reducing lock levels was safe, which Robert, Tom and others were slightly uncertain about. Is there anything different here from work in my original patch series? -- Simon Riggs 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
Re: [HACKERS] Reducing lock strength of adding foreign keys
On 10/28/2014 01:33 AM, Noah Misch wrote: ALTER TRIGGER is not bad; like you say, change pg_get_triggerdef_worker() the way commit e5550d5 changed pg_get_constraintdef_worker(). DROP TRIGGER is more difficult. pg_constraint.tgqual of a dropped trigger may reference other dropped objects, which calls for equipping get_rule_expr() to use the transaction snapshot. That implicates quite a bit of ruleutils.c code. I started looking into this again and fixed pg_get_constraintdef_worker() as suggested. But I have no idea how to fix get_rule_expr() since it relies on doing lookups in the catcache. Replacing these with uncached lookups sounds like it could cause quite some slowdown. Any ideas? Indexes should suffer from the same problems since they too have emay contain expressions but they seem to solve this by having a higher lock level on DROP INDEX, but I do wonder about the CONCURRENTLY case. By the way, unless I am mistaken there is currently no protection against having a concurrent ALTER FUNCTION ... RENAME mess up what is dumped in by pg_get_triggerdef(). -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
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. - GUL -- View this message in context: http://postgresql.1045698.n5.nabble.com/Reducing-lock-strength-of-adding-foreign-keys-tp5823894p5824376.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
Thanks for weighing in, Noah. On Sat, Oct 25, 2014 at 2:00 PM, Noah Misch n...@leadboat.com wrote: 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. pg_get_triggerdef() is fine as-is with concurrent CREATE TRIGGER. The pg_get_constraintdef() change arose to ensure a consistent result when concurrent ALTER TABLE VALIDATE CONSTRAINT mutates a constraint definition. (Reducing the lock level of DROP TRIGGER or ALTER TRIGGER, however, would create the analogous problem for pg_get_triggerdef().) Maybe so, but I'd favor changing it anyway and getting it over with. The current situation seems to have little to recommend it; moreover, it would be nice, if it's possible and safe, to weaken the lock levels for all three of those commands at the same time. Do you see any hazards for ALTER or DROP that do not exist for CREATE? -- 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
Re: [HACKERS] Reducing lock strength of adding foreign keys
On Sun, Oct 26, 2014 at 9:48 PM, Andreas Karlsson andr...@proxel.se wrote: Agreed.. But I think reducing the lock level of the secondary table is much more important than doing the same for the primary table due to the case where the secondary table is an existing table which is hit by a workload of long running queries and DML while the primary is a new table which is added now. In my dream world I could add the new table without any disruption at all of queries using the secondary table, no matter the duration of the transaction adding the table (barring insertion of actual data into the primary table, which would take row locks). That would indeed be nice, but it doesn't seem very practical, because the parent needs triggers, too: if you try to delete a row from the parent, or update the key, it's got to go look at the child and see whether there are rows against the old value. Then it's got to either update those rows, or null out the value, or throw an error. Regardless of which of those things it does (which depends on the ON DELETE and ON UPDATE settings you choose), it's hard to imagine that it would be a good idea for any of those things to start happening in the middle of a transaction or statement. So, let's take what we can get. :-) -- 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
Re: [HACKERS] Reducing lock strength of adding foreign keys
On Mon, Oct 27, 2014 at 08:24:15AM -0400, Robert Haas wrote: On Sat, Oct 25, 2014 at 2:00 PM, Noah Misch n...@leadboat.com wrote: 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. pg_get_triggerdef() is fine as-is with concurrent CREATE TRIGGER. The pg_get_constraintdef() change arose to ensure a consistent result when concurrent ALTER TABLE VALIDATE CONSTRAINT mutates a constraint definition. (Reducing the lock level of DROP TRIGGER or ALTER TRIGGER, however, would create the analogous problem for pg_get_triggerdef().) Maybe so, but I'd favor changing it anyway and getting it over with. The current situation seems to have little to recommend it; moreover, it would be nice, if it's possible and safe, to weaken the lock levels for all three of those commands at the same time. Do you see any hazards for ALTER or DROP that do not exist for CREATE? ALTER TRIGGER is not bad; like you say, change pg_get_triggerdef_worker() the way commit e5550d5 changed pg_get_constraintdef_worker(). DROP TRIGGER is more difficult. pg_constraint.tgqual of a dropped trigger may reference other dropped objects, which calls for equipping get_rule_expr() to use the transaction snapshot. That implicates quite a bit of ruleutils.c code. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
On 10/24/2014 06:07 PM, Robert Haas wrote: I think instead of focusing on foreign keys, we should rewind a bit and think about the locking level required to add a trigger. Agreed. 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. Thanks for the info. This is just the kind of issues I was worrying about. 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. Would not ShareLock give the same result, except for also allowing concurrent CREATE INDEX and concurrent other CREATE TRIGGER which does not look dangerous to me? From a user point of view ShareRowExclusiveLock should be as useful as ShareLock. 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. Agreed.. But I think reducing the lock level of the secondary table is much more important than doing the same for the primary table due to the case where the secondary table is an existing table which is hit by a workload of long running queries and DML while the primary is a new table which is added now. In my dream world I could add the new table without any disruption at all of queries using the secondary table, no matter the duration of the transaction adding the table (barring insertion of actual data into the primary table, which would take row locks). This is just a dream scenario though, and focusing on triggers is indeed the reasonable goal for 9.5. -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
On Fri, Oct 24, 2014 at 12:07:42PM -0400, Robert Haas wrote: I think instead of focusing on foreign keys, we should rewind a bit and think about the locking level required to add a trigger. Agreed. 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. pg_get_triggerdef() is fine as-is with concurrent CREATE TRIGGER. The pg_get_constraintdef() change arose to ensure a consistent result when concurrent ALTER TABLE VALIDATE CONSTRAINT mutates a constraint definition. (Reducing the lock level of DROP TRIGGER or ALTER TRIGGER, however, would create the analogous problem for pg_get_triggerdef().) 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. +1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
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
Re: [HACKERS] Reducing lock strength of adding foreign keys
On 10/22/2014 04:13 PM, Tom Lane wrote: Andreas Karlsson andr...@proxel.se writes: I have attached a proof of concept patch which reduces the lock strength to ShareLock. You're kidding, right? ShareLock isn't even self-exclusive. Why would it have to be self-exclusive? As far as I know we only need to ensure that nobody changes the rows while we add the trigger. Adding multiple triggers concurrently should not pose a problem unless I am missing something (which I probably am). Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reducing lock strength of adding foreign keys
Andreas Karlsson andr...@proxel.se writes: I have attached a proof of concept patch which reduces the lock strength to ShareLock. You're kidding, right? ShareLock isn't even self-exclusive. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
