From 1329d9cff8ff85f6233bbf442d64a0f917c802e3 Mon Sep 17 00:00:00 2001 From: Shayon Mukherjee Date: Wed, 8 Oct 2025 16:57:04 -0400 Subject: [PATCH v1] Reduce lock level for FK/trigger drops to allow concurrent reads Dropping a foreign key constraint or a table that owns an FK currently blocks reads on the other table due to AccessExclusiveLock taken while removing the FK's internal RI triggers and constraint metadata. In busy systems, this brief full-read outage can cause user-visible timeouts for otherwise read-only traffic. This commit reduces the relation-level lock from AccessExclusiveLock to ShareRowExclusiveLock in three places: 1. RemoveConstraintById(): when opening the constraint's owning relation 2. RemoveTriggerById(): when opening the trigger's relation 3. dropconstraint_internal(): when opening the referenced table for FK drops ShareRowExclusiveLock allows SELECT queries to proceed while still blocking writers (INSERT/UPDATE/DELETE/most DDL) during the window where RI triggers, constraint rows, and relcache entries are removed and invalidated. The table being dropped or directly altered still acquires AccessExclusiveLock as before. Only the "other" tables (such as referenced tables in FK relationships) see the reduced lock level. Examples of affected operations: - ALTER TABLE fktable DROP CONSTRAINT: now takes ShareRowExclusive on both fktable and the referenced pktable, allowing SELECTs on pktable to proceed - DROP TABLE fktable: now takes ShareRowExclusive on the referenced pktable for RI trigger removal, allowing SELECTs on pktable to proceed - ALTER TABLE ... ALTER COLUMN TYPE: when rebuilding FKs, now takes ShareRowExclusive on referenced tables Correctness is preserved because: - Writers are serialized: ShareRowExclusive conflicts with RowExclusiveLock and stronger, so no DML can create new RI trigger events during removal - Relcache invalidation at commit ensures metadata changes become visible to subsequent queries - Prepared plans continue to work (revalidate on next execution) Updated isolation tests to reflect new lock behavior: - detach-partition-concurrently-4: added writer-blocking permutations - fk-drop-constraint-concurrency (new): covers regular FKs, DROP TABLE, self-referential FKs, and relcache invalidation --- doc/src/sgml/ref/alter_table.sgml | 9 ++ src/backend/catalog/pg_constraint.c | 7 +- src/backend/commands/tablecmds.c | 11 +-- src/backend/commands/trigger.c | 6 +- .../detach-partition-concurrently-4.out | 34 ++++++-- .../fk-drop-constraint-concurrency.out | 85 +++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../detach-partition-concurrently-4.spec | 5 ++ .../specs/fk-drop-constraint-concurrency.spec | 69 +++++++++++++++ 9 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 src/test/isolation/expected/fk-drop-constraint-concurrency.out create mode 100644 src/test/isolation/specs/fk-drop-constraint-concurrency.spec diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 8867da6c69..646578a460 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -623,6 +623,15 @@ WITH ( MODULUS numeric_literal, REM If IF EXISTS is specified and the constraint does not exist, no error is thrown. In this case a notice is issued instead. + + For foreign key constraints, this acquires ACCESS + EXCLUSIVE lock on the table owning the constraint (as with + other ALTER TABLE forms), but only SHARE + ROW EXCLUSIVE lock on the referenced table. This allows + concurrent reads on the referenced table while the constraint is being + dropped. Other constraint types acquire ACCESS + EXCLUSIVE lock on the table being altered. + diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 6002fd0002..13b21b6ee1 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -913,10 +913,11 @@ RemoveConstraintById(Oid conId) Relation rel; /* - * If the constraint is for a relation, open and exclusive-lock the - * relation it's for. + * If the constraint is for a relation, open and lock the relation. + * ShareRowExclusiveLock allows readers to proceed while blocking + * concurrent writers during constraint removal. */ - rel = table_open(con->conrelid, AccessExclusiveLock); + rel = table_open(con->conrelid, ShareRowExclusiveLock); /* * We need to update the relchecks count if it is a check constraint diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fc89352b66..6d31fd8a38 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14165,7 +14165,8 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha * and check that that's not in use, just as we've already done for the * constrained table (else we might, eg, be dropping a trigger that has * unfired events). But we can/must skip that in the self-referential - * case. + * case. Use ShareRowExclusive to allow concurrent reads on the referenced + * table. */ if (con->contype == CONSTRAINT_FOREIGN && con->confrelid != RelationGetRelid(rel)) @@ -14173,7 +14174,7 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha Relation frel; /* Must match lock taken by RemoveTriggerById: */ - frel = table_open(con->confrelid, AccessExclusiveLock); + frel = table_open(con->confrelid, ShareRowExclusiveLock); CheckAlterTableIsSafe(frel); table_close(frel, NoLock); } @@ -15474,11 +15475,11 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) /* * When rebuilding another table's constraint that references the * table we're modifying, we might not yet have any lock on the other - * table, so get one now. We'll need AccessExclusiveLock for the DROP - * CONSTRAINT step, so there's no value in asking for anything weaker. + * table, so get one now. Use ShareRowExclusiveLock to block writers + * but allow readers during the DROP CONSTRAINT step. */ if (relid != tab->relid) - LockRelationOid(relid, AccessExclusiveLock); + LockRelationOid(relid, ShareRowExclusiveLock); ATPostAlterTypeParse(oldId, relid, confrelid, (char *) lfirst(def_item), diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 579ac8d76a..fae0cc9a92 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1315,11 +1315,13 @@ RemoveTriggerById(Oid trigOid) elog(ERROR, "could not find tuple for trigger %u", trigOid); /* - * Open and exclusive-lock the relation the trigger belongs to. + * Open and lock the relation the trigger belongs to. + * ShareRowExclusiveLock allows reads while blocking concurrent writes + * during trigger removal. */ relid = ((Form_pg_trigger) GETSTRUCT(tup))->tgrelid; - rel = table_open(relid, AccessExclusiveLock); + rel = table_open(relid, ShareRowExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION && rel->rd_rel->relkind != RELKIND_VIEW && diff --git a/src/test/isolation/expected/detach-partition-concurrently-4.out b/src/test/isolation/expected/detach-partition-concurrently-4.out index 79b29ae4c1..86e0a7ff54 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-4.out +++ b/src/test/isolation/expected/detach-partition-concurrently-4.out @@ -254,11 +254,8 @@ a 2 (1 row) -step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; -step s1cancel: select pg_cancel_backend(pid) from d4_pid; -step s2detach: <... completed> -ERROR: canceling statement due to user request -step s1cancel: <... completed> +step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; +step s1cancel: select pg_cancel_backend(pid) from d4_pid; pg_cancel_backend ----------------- t @@ -278,10 +275,9 @@ a 2 (1 row) -step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; +step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; step s1updcur: update d4_fk set a = 1 where current of f; ERROR: insert or update on table "d4_fk" violates foreign key constraint "d4_fk_a_fkey" -step s2detach: <... completed> step s1c: commit; starting permutation: s2snitch s1brr s1declare2 s1fetchone s1updcur s2detach s1c @@ -360,6 +356,30 @@ step s3commit: commit; step s1c: commit; step s2detach: <... completed> +starting permutation: s2snitch s1b s1write s2detach s1cancel s1c +step s2snitch: insert into d4_pid select pg_backend_pid(); +step s1b: begin; +step s1write: update d4_fk set a = a where a = 2; +step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; +step s1cancel: select pg_cancel_backend(pid) from d4_pid; +step s2detach: <... completed> +ERROR: canceling statement due to user request +step s1cancel: <... completed> +pg_cancel_backend +----------------- +t +(1 row) + +step s1c: commit; + +starting permutation: s2snitch s1b s1write s2detach s1c +step s2snitch: insert into d4_pid select pg_backend_pid(); +step s1b: begin; +step s1write: update d4_fk set a = a where a = 2; +step s2detach: alter table d4_primary detach partition d4_primary1 concurrently; +step s1c: commit; +step s2detach: <... completed> + starting permutation: s2snitch s1brr s1s s2detach s1cancel s1noop s3vacfreeze s1s s1insert s1c step s2snitch: insert into d4_pid select pg_backend_pid(); step s1brr: begin isolation level repeatable read; diff --git a/src/test/isolation/expected/fk-drop-constraint-concurrency.out b/src/test/isolation/expected/fk-drop-constraint-concurrency.out new file mode 100644 index 0000000000..8e416e209f --- /dev/null +++ b/src/test/isolation/expected/fk-drop-constraint-concurrency.out @@ -0,0 +1,85 @@ +Parsed test spec with 2 sessions + +starting permutation: s1b s1s s2drop s1c +step s1b: begin; +step s1s: select * from pk; +id +-- + 1 +(1 row) + +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; + +starting permutation: s1b s1wfk s2drop s1c +step s1b: begin; +step s1wfk: update fk set ref = ref where id = 1; +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; +step s2drop: <... completed> + +starting permutation: s1b s1wpk s2drop s1c +step s1b: begin; +step s1wpk: update pk set id = id where id = 1; +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1c: commit; +step s2drop: <... completed> + +starting permutation: s1prep s1b s1exec s2drop s1exec s1c +step s1prep: prepare q as select count(*) from pk; +step s1b: begin; +step s1exec: execute q; +count +----- + 1 +(1 row) + +step s2drop: alter table fk drop constraint fk_ref_fkey; +step s1exec: execute q; +count +----- + 1 +(1 row) + +step s1c: commit; + +starting permutation: s1b s1s s2drop_table s1c +step s1b: begin; +step s1s: select * from pk; +id +-- + 1 +(1 row) + +step s2drop_table: drop table fk; +step s1c: commit; + +starting permutation: s1b s1s_self s2drop_self s1c +step s1b: begin; +step s1s_self: select * from fkself; +id|parent +--+------ + 1| +(1 row) + +step s2drop_self: alter table fkself drop constraint fkself_parent_fkey; +step s1c: commit; +step s2drop_self: <... completed> + +starting permutation: s1b s1w_self s2drop_self s1c +step s1b: begin; +step s1w_self: update fkself set id = id where id = 1; +step s2drop_self: alter table fkself drop constraint fkself_parent_fkey; +step s1c: commit; +step s2drop_self: <... completed> + +starting permutation: s1b s1s s2alter_type s1c +step s1b: begin; +step s1s: select * from pk; +id +-- + 1 +(1 row) + +step s2alter_type: alter table fk_alttype alter column ref type bigint; +step s1c: commit; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 5afae33d37..987a3c0a1a 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -120,3 +120,4 @@ test: serializable-parallel-2 test: serializable-parallel-3 test: matview-write-skew test: lock-nowait +test: fk-drop-constraint-concurrency diff --git a/src/test/isolation/specs/detach-partition-concurrently-4.spec b/src/test/isolation/specs/detach-partition-concurrently-4.spec index 829c82248f..dcd4791a2f 100644 --- a/src/test/isolation/specs/detach-partition-concurrently-4.spec +++ b/src/test/isolation/specs/detach-partition-concurrently-4.spec @@ -27,6 +27,7 @@ session s1 step s1b { begin; } step s1brr { begin isolation level repeatable read; } step s1s { select * from d4_primary; } +step s1write { update d4_fk set a = a where a = 2; } step s1cancel { select pg_cancel_backend(pid) from d4_pid; } step s1noop { } step s1insert { insert into d4_fk values (1); } @@ -77,6 +78,10 @@ permutation s2snitch s1b s1s s2detach s3insert s1c permutation s2snitch s1b s1s s2detach s3brr s3insert s3commit s1cancel(s2detach) s1c permutation s2snitch s1b s1s s2detach s3brr s3insert s3commit s1c +# Explicit coverage that a writer on the referencing table blocks detach +permutation s2snitch s1b s1write s2detach s1cancel(s2detach) s1c +permutation s2snitch s1b s1write s2detach s1c + # Try one where we VACUUM FREEZE pg_inherits (to verify that xmin change is # handled correctly). permutation s2snitch s1brr s1s s2detach s1cancel(s2detach) s1noop s3vacfreeze s1s s1insert s1c diff --git a/src/test/isolation/specs/fk-drop-constraint-concurrency.spec b/src/test/isolation/specs/fk-drop-constraint-concurrency.spec new file mode 100644 index 0000000000..697d95eb14 --- /dev/null +++ b/src/test/isolation/specs/fk-drop-constraint-concurrency.spec @@ -0,0 +1,69 @@ +# Regular-table analogue of detach-partition-concurrently behavior for FK drop. +# +# This spec exercises reduced relation-level locking (ShareRowExclusive) for +# FK/trigger removal: +# 1) SELECTs on the referenced table (pk) proceed while DROP CONSTRAINT runs. +# 2) Writers (UPDATE) on the referencing (fk) table block the FK drop. +# 3) Writers (UPDATE) on the referenced (pk) table block the FK drop. +# 4) Prepared SELECTs (plan + relcache) continue to work across FK drop. +# 5) DROP TABLE fktable doesn't block SELECTs on the referenced table. +# 6) Self-referential FK drops don't block SELECTs on the table. +# 7) ALTER COLUMN TYPE with FK rebuild doesn't block SELECTs on referenced table. + +setup { + drop table if exists pk, fk, fkself, fk_alttype; + create table pk(id int primary key); + create table fk(id int primary key, ref int references pk(id)); + create table fkself(id int primary key, parent int references fkself(id)); + create table fk_alttype(id int primary key, ref int references pk(id)); + insert into pk values (1); + insert into fk values (1, 1); + insert into fkself values (1, NULL); + insert into fk_alttype values (1, 1); +} + +teardown { + drop table if exists fk, fkself, fk_alttype, pk; +} + +session s1 +step s1b { begin; } +step s1s { select * from pk; } +step s1s_self { select * from fkself; } +step s1wfk { update fk set ref = ref where id = 1; } +step s1wpk { update pk set id = id where id = 1; } +step s1w_self { update fkself set id = id where id = 1; } +step s1c { commit; } +step s1prep { prepare q as select count(*) from pk; } +step s1exec { execute q; } + +session s2 +step s2drop { alter table fk drop constraint fk_ref_fkey; } +step s2drop_table { drop table fk; } +step s2drop_self { alter table fkself drop constraint fkself_parent_fkey; } +step s2alter_type { alter table fk_alttype alter column ref type bigint; } + +# (1) Readers do not block FK drop +permutation s1b s1s s2drop s1c + +# (2) Writer on referencing table blocks FK drop +permutation s1b s1wfk s2drop s1c + +# (3) Writer on referenced table blocks FK drop +permutation s1b s1wpk s2drop s1c + +# (4) Prepared plan remains usable across FK drop; relcache invalidation ensures correctness +permutation s1prep s1b s1exec s2drop s1exec s1c + +# (5) DROP TABLE fktable does not block SELECTs on referenced table +permutation s1b s1s s2drop_table s1c + +# (6) Self-referential FK drop: ALTER TABLE takes AccessExclusive on the table +# being altered, so even SELECTs block (different from non-self-ref case) +permutation s1b s1s_self s2drop_self s1c + +# (7) Writer on self-referential FK table blocks drop of self-referential FK +permutation s1b s1w_self s2drop_self s1c + +# (8) ALTER COLUMN TYPE rebuilding FK: SELECTs on referenced table proceed +permutation s1b s1s s2alter_type s1c -- 2.39.5 (Apple Git-154)