Обсуждение: Proposal for discussions: Allow reads to proceed during FK/trigger drops by reducing relation-level lock from AccessExclusive to ShareRowExclusive

Hello hackers,

I stumbled upon a case and wanted to raise a quick thread to get some feedback on this.

Basically, dropping a foreign key constraint or a table that owns an FK currently blocks reads on the other table due to AccessExclusive locks taken while removing the FK’s internal triggers and constraint metadata. In busy systems, this short full-read outage can cause user-visible timeouts for otherwise read-only traffic. Similar topic discussed here as well [1]

Example setup
- pktable(id primary key)
- fktable(id primary key, fk references pktable(id))

Where reads get blocked today
- ALTER TABLE fktable DROP CONSTRAINT fkname:
  - The FK lives on fktable, and its RI action triggers live on pktable.
  - Both tables see AccessExclusive-level effects during removal, so SELECTs on either can be blocked while the FK and triggers are dropped.

- DROP TABLE fktable:
  - fktable is dropped with AccessExclusive (expected).
  - While removing RI action triggers on pktable, pktable also sees an AccessExclusive lock, so SELECTs on pktable can be blocked even though pktable is not being dropped.

- DROP TABLE pktable CASCADE:
  - pktable is dropped with AccessExclusive (expected).
  - FK removal on fktable (check triggers) also introduces an AccessExclusive effect on fktable, so SELECTs on fktable can be blocked.

Proposal
I wanted to see if we could reduce the relation-level lock used specifically for FK/trigger removal from AccessExclusive to ShareRowExclusive. This keeps readers moving while still blocking writers during the small window where RI triggers/constraint rows are removed and relcache is invalidated. Ideally, with a change like this `ALTER TABLE fktable DROP CONSTRAINT fkname` should take ShareRowExclusive for fktable and pktable, and `DROP TABLE fktable` should take ShareRowExclusive for fktable and AccessExclusive for pktable.

Next, dropping fktable would take ShareRowExclusive for fktable and AccessExclusive for pktable. And DROP TABLE pktable CASCADE would take AccessExclusive for pktable and ShareRowExclusive for fktable.

My understanding is that the table being dropped still uses AccessExclusive; reads/writes on that table remain blocked as today while it updates relcache, updating metadata/catalog entries and other cleanup tasks.

Also, I believe ShareRowExclusive would still serialize writers, so there should be no window where DML can bypass enforcement during removal. So this change should not affect correctness?

I just attached a small patch to get the idea across. If folks think this direction makes sense, I am happy to work on a more complete patch as well. If I’ve missed any critical cases that truly need AccessExclusive at the relation level during FK/trigger removal, especially around partitions, pending trigger events, hot standby, logical decoding, event triggers or something else perhaps please let me know.

[1] https://www.postgresql.org/message-id/flat/2bdf0fe5679f44ebf56890dfd5c5ead21a0c0bc3.camel%40cybertec.at#c50baf640c680c9e3f4def34b565d3b9

Thanks for your time and feedback.
Shayon

Shayon Mukherjee <shayonj@gmail.com> writes:
> Following up on the previous thread - I took a stab at trying to see what a
> full patch for the proposal to reduce lock levels during FK/trigger drops
> would look like, and this is what I ended up with.

I don't think this is safe, at least not for FK removal.  There's a
comment in AlterTableGetLockLevel explaining why:

                /*
                 * Removing constraints can affect SELECTs that have been
                 * optimized assuming the constraint holds true. See also
                 * CloneFkReferenced.
                 */

Adding a foreign key can (and I think does) take a lesser lock,
because the additional constraint won't invalidate any proofs the
optimizer may have made beforehand.  But dropping one seems
problematic.

Another issue is that the proposed patch looks like it reduces
the locking level for more types of constraints than just FKs.
That would require further analysis, but I believe that (for
example) dropping a unique constraint likewise risks breaking
existing query plans, even when they aren't directly using that
index.