Postgres attach partition: AccessExclusive lock set on different tables depending on how attaching is performed

On 10/20/24 04:31, user wrote:

Hello,
I was reading all the tips that could make the attach partition
operation seamless.
https://www.postgresql.org/docs/current/ddl-partitioning.html
<https://www.postgresql.org/docs/current/ddl-partitioning.html&gt; There is
a mention about check constraint that could be places before the attach
process. But to minimise the time when AccessExclusive lock is held on
my table, I wanted to push it further and also add indexes and foreign
keys BEFORE the attach command is invoked.
And here is a problem. When I run the attach command without foreign
keys being present beforehand on a table, there is only AccessExclusive
lock on a table I attach partition to.
BUT if my table to-be-attached has a foreign key constraint already,
then the referenced table will get the ExclusiveLock! I do not
understand why is it needed, the constraint already exists...

The reproduction: ( Postgres Version 14 )

CREATE TABLE refs (
                    id integer primary key,
                    did integer
                );
CREATE TABLE films (
                    id integer,
                    code char(5) ,
                    title varchar(40) NOT NULL,
                    did integer NOT NULL references refs(id)
                    )
                    partition by list (code);

insert into refs values (5, 5)
create table films_partition (LIKE films INCLUDING ALL)
case 1: films_partition does not have a foreign key added before the attach
BEGIN;
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code = 'dr');
ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')
keep the transaction running...

check the locks:

select relname, mode
                from pg_locks l
                    join pg_class c on (relation = c.oid)
                    join pg_namespace nsp on (c.relnamespace = nsp.oid);
films relname, ShareUpdateExclusiveLock mode
films_partition relname, AccessShareLock mode
films_partition relname, RowExclusiveLock mode
films_partition relname, ShareRowExclusiveLock mode
films_partition relname, AccessExclusiveLock mode
refs relname, AccessShareLock mode
refs relname, RowShareLock mode
refs relname, ShareRowExclusiveLock mode

No AccessExclusive lock on "refs" table!

case 2: films_partition does have the foreign key contrain
 BEGIN;
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code = 'dr');
alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);
ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')
keep the transaction running...

check the locks:

films relname, ShareUpdateExclusiveLock mode
films_partition relname, AccessShareLock mode
films_partition relname, RowExclusiveLock mode
films_partition relname, ShareRowExclusiveLock mode
films_partition relname, AccessExclusiveLock mode
refs relname, AccessShareLock mode
refs relname, RowShareLock mode
refs relname, ShareRowExclusiveLock mode
refs relname, AccessExclusiveLock mode

There is AccessExclusiveLock on "refs" table!

Conclusion
I really don't want the "attach partition" to take too much time, so I
want to have all the constraints added before it is run. And indeed, the
time is reduced. But this additional lock now increases the chance of
deadlocks, as AccessExclusive locks are grabbed on many tables
referenced by foreing keys. Is there anything I can do better? Whi is it
that attach_partition adds a foreign key without additional
AccessExclusive lock,

https://www.postgresql.org/docs/current/sql-altertable.html

ATTACH PARTITION

[...]

"Currently FOREIGN KEY constraints are not considered. "

but this lock is required when the constrint

already exists?

Because I am pretty sure it is due to this statement:

alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);

Try:

BEGIN;
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code = 'dr');
alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);
COMMIT;

Then:

ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')

Also from

https://www.postgresql.org/docs/current/sql-altertable.html

Take a look at:

ADD table_constraint [ NOT VALID ]

...

"Normally, this form will cause a scan of the table to verify that all
existing rows in the table satisfy the new constraint. But if the NOT
VALID option is used, this potentially-lengthy scan is skipped. The
constraint will still be enforced against subsequent inserts or updates
(that is, they'll fail unless there is a matching row in the referenced
table, in the case of foreign keys, or they'll fail unless the new row
matches the specified check condition). But the database will not assume
that the constraint holds for all rows in the table, until it is
validated by using the VALIDATE CONSTRAINT option. See Notes below for
more information about using the NOT VALID option."

Regards!

--
Adrian Klaver
adrian.klaver@aklaver.com

** forwarding to mailing list, forgot to add header

Thanks for answering.
I think one misunderstanding happened.
The parent table has the foreign key constraint.
So attach partition will add this constraint for table being attached. (How
this compares to foreign keys not being considered, not sure).

Why is it that attach_partition does not require exclusive lock when
creating a constraint automatically?

What is more, you have provided a quote that states the lock is needed
because the table needs to be checked that all entries comply with the NEW
constraint.

Well it is not new when I manually create it before I attach.
It is new when I run attach command without previous manual constraint
creation, but then the lock is not created.

On Sun, 20 Oct 2024, 18:23 Adrian Klaver, <adrian.klaver@aklaver.com> wrote:

On 10/21/24 1:40 AM, user wrote:

** forwarding to mailing list, forgot to add header

Thanks for answering.
I think one misunderstanding happened.
The parent table has the foreign key constraint.
So attach partition will add this constraint for table being attached.
(How this compares to foreign keys not being considered, not sure).

Why is it that attach_partition does not require exclusive lock when
creating a constraint automatically?

What is more, you have provided a quote that states the lock is needed
because the table needs to be checked that all entries comply with the
NEW constraint.

Well it is not new when I manually create it before I attach.
It is new when I run attach command without previous manual constraint
creation, but then the lock is not created.

1) Case 1

test=# \d films
Partitioned table "public.films"
Column | Type | Collation | Nullable | Default
--------+-----------------------+-----------+----------+---------
id | integer | | |
code | character(5) | | |
title | character varying(40) | | not null |
did | integer | | not null |
Partition key: LIST (code)
Foreign-key constraints:
"films_did_fkey" FOREIGN KEY (did) REFERENCES refs(id)
Number of partitions: 0

create table films_partition (LIKE films INCLUDING ALL);
CREATE TABLE

test=# \d+ films_partition
Table "public.films_partition"
Column | Type | Collation | Nullable | Default |
Storage | Compression | Stats target | Description
--------+-----------------------+-----------+----------+---------+----------+-------------+--------------+-------------
id | integer | | | |
plain | | |
code | character(5) | | | |
extended | | |
title | character varying(40) | | not null | |
extended | | |
did | integer | | not null | |
plain | | |
Access method: heap

ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr');
ALTER TABLE

test=# \d+ films
Partitioned table "public.films"
Column | Type | Collation | Nullable | Default |
Storage | Compression | Stats target | Description
--------+-----------------------+-----------+----------+---------+----------+-------------+--------------+-------------
id | integer | | | |
plain | | |
code | character(5) | | | |
extended | | |
title | character varying(40) | | not null | |
extended | | |
did | integer | | not null | |
plain | | |
Partition key: LIST (code)
Foreign-key constraints:
"films_did_fkey" FOREIGN KEY (did) REFERENCES refs(id)
Partitions: films_partition FOR VALUES IN ('dr ')

test=# \d+ films_partition
Table "public.films_partition"
Column | Type | Collation | Nullable | Default |
Storage | Compression | Stats target | Description
--------+-----------------------+-----------+----------+---------+----------+-------------+--------------+-------------
id | integer | | | |
plain | | |
code | character(5) | | | |
extended | | |
title | character varying(40) | | not null | |
extended | | |
did | integer | | not null | |
plain | | |
Partition of: films FOR VALUES IN ('dr ')
Partition constraint: ((code IS NOT NULL) AND (code = 'dr
'::character(5)))
Check constraints:
"check_code" CHECK (code = 'dr'::bpchar)
Foreign-key constraints:
TABLE "films" CONSTRAINT "films_did_fkey" FOREIGN KEY (did)
REFERENCES refs(id)
Access method: heap

2) Case 2

create table films_partition (LIKE films INCLUDING ALL);

insert into films_partition values (1, 'dr', 'musician',5);

alter table films_partition add constraint check_code check (code = 'dr');

alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);

test=# \d films_partition
Table "public.films_partition"
Column | Type | Collation | Nullable | Default
--------+-----------------------+-----------+----------+---------
id | integer | | |
code | character(5) | | |
title | character varying(40) | | not null |
did | integer | | not null |
Check constraints:
"check_code" CHECK (code = 'dr'::bpchar)
Foreign-key constraints:
"fk_did" FOREIGN KEY (did) REFERENCES refs(id)

****Note the FK definition***

test=# ALTER TABLE films ATTACH PARTITION films_partition for values in
('dr');

test=# \d films_partition
Table "public.films_partition"
Column | Type | Collation | Nullable | Default
--------+-----------------------+-----------+----------+---------
id | integer | | |
code | character(5) | | |
title | character varying(40) | | not null |
did | integer | | not null |
Partition of: films FOR VALUES IN ('dr ')
Check constraints:
"check_code" CHECK (code = 'dr'::bpchar)
Foreign-key constraints:
TABLE "films" CONSTRAINT "films_did_fkey" FOREIGN KEY (did)
REFERENCES refs(id)

****Note the FK definition***

What you are seeing is the locking for

alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);

At this point films_partition is a stand alone table that you are
creating a FK back to refs. The ALTER TABLE films_partition ADD
CONSTRAINT command has no knowledge of the target table you are going to
attach films_partition to. When you do the ATTACH then a new FK is
created just the same as in Case 1.

On Sun, 20 Oct 2024, 18:23 Adrian Klaver, <adrian.klaver@aklaver.com
<mailto:adrian.klaver@aklaver.com>> wrote:

On 10/20/24 04:31, user wrote:

Hello,
I was reading all the tips that could make the attach partition
operation seamless.
https://www.postgresql.org/docs/current/ddl-partitioning.html

<https://www.postgresql.org/docs/current/ddl-partitioning.html&gt;

<https://www.postgresql.org/docs/current/ddl-partitioning.html

<https://www.postgresql.org/docs/current/ddl-partitioning.html&gt;&gt;
There is

a mention about check constraint that could be places before the

attach

process. But to minimise the time when AccessExclusive lock is

held on

my table, I wanted to push it further and also add indexes and

foreign

keys BEFORE the attach command is invoked.
And here is a problem. When I run the attach command without foreign
keys being present beforehand on a table, there is only

AccessExclusive

lock on a table I attach partition to.
BUT if my table to-be-attached has a foreign key constraint already,
then the referenced table will get the ExclusiveLock! I do not
understand why is it needed, the constraint already exists...

The reproduction: ( Postgres Version 14 )

CREATE TABLE refs (
                      id integer primary key,
                      did integer
                  );
CREATE TABLE films (
                      id integer,
                      code char(5) ,
                      title varchar(40) NOT NULL,
                      did integer NOT NULL references refs(id)
                      )
                      partition by list (code);

insert into refs values (5, 5)
create table films_partition (LIKE films INCLUDING ALL)
case 1: films_partition does not have a foreign key added before

the attach

BEGIN;
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code

= 'dr');

ALTER TABLE films ATTACH PARTITION films_partition for values in

('dr')

keep the transaction running...

check the locks:

select relname, mode
                  from pg_locks l
                      join pg_class c on (relation = c.oid)
                      join pg_namespace nsp on (c.relnamespace =

nsp.oid);

films relname, ShareUpdateExclusiveLock mode
films_partition relname, AccessShareLock mode
films_partition relname, RowExclusiveLock mode
films_partition relname, ShareRowExclusiveLock mode
films_partition relname, AccessExclusiveLock mode
refs relname, AccessShareLock mode
refs relname, RowShareLock mode
refs relname, ShareRowExclusiveLock mode

No AccessExclusive lock on "refs" table!

case 2: films_partition does have the foreign key contrain
   BEGIN;
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code

= 'dr');

alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);
ALTER TABLE films ATTACH PARTITION films_partition for values in

('dr')

keep the transaction running...

check the locks:

films relname, ShareUpdateExclusiveLock mode
films_partition relname, AccessShareLock mode
films_partition relname, RowExclusiveLock mode
films_partition relname, ShareRowExclusiveLock mode
films_partition relname, AccessExclusiveLock mode
refs relname, AccessShareLock mode
refs relname, RowShareLock mode
refs relname, ShareRowExclusiveLock mode
refs relname, AccessExclusiveLock mode

There is AccessExclusiveLock on "refs" table!

Conclusion
I really don't want the "attach partition" to take too much time,

so I

want to have all the constraints added before it is run. And

indeed, the

time is reduced. But this additional lock now increases the

chance of

deadlocks, as AccessExclusive locks are grabbed on many tables
referenced by foreing keys. Is there anything I can do better?

Whi is it

that attach_partition adds a foreign key without additional
AccessExclusive lock,

https://www.postgresql.org/docs/current/sql-altertable.html
<https://www.postgresql.org/docs/current/sql-altertable.html&gt;

ATTACH PARTITION

[...]

"Currently FOREIGN KEY constraints are not considered. "

  but this lock is required when the constrint

already exists?

Because I am pretty sure it is due to this statement:

alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);

Try:

BEGIN;
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code =
'dr');
alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);
COMMIT;

Then:

ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')

Also from

https://www.postgresql.org/docs/current/sql-altertable.html
<https://www.postgresql.org/docs/current/sql-altertable.html&gt;

Take a look at:

ADD table_constraint [ NOT VALID ]

...

"Normally, this form will cause a scan of the table to verify that all
existing rows in the table satisfy the new constraint. But if the NOT
VALID option is used, this potentially-lengthy scan is skipped. The
constraint will still be enforced against subsequent inserts or updates
(that is, they'll fail unless there is a matching row in the referenced
table, in the case of foreign keys, or they'll fail unless the new row
matches the specified check condition). But the database will not
assume
that the constraint holds for all rows in the table, until it is
validated by using the VALIDATE CONSTRAINT option. See Notes below for
more information about using the NOT VALID option."

Regards!

--
Adrian Klaver
adrian.klaver@aklaver.com <mailto:adrian.klaver@aklaver.com>

--
Adrian Klaver
adrian.klaver@aklaver.com

Hello Adrian,
My apology for answering so late.

Indeed I have not splitted my transactions correctly in my first example.
BUT, if you change the syntaxt so that attach is performed in its own
transaction, result is the same. Try it out!

Also you have told me that I am seeing ExclusiveLock on refs table because
it is from foreign key constraint. Well this is incorrect, foreign key does
not take the ExclusiveLock in any situation.
https://pglocks.org/?pgcommand=ALTER%20TABLE%20ADD%20FOREIGN%20KEY%20(CHILD)

Could you try again with only attach being in its own transaction? You
should reproduce it.
Regards

On Mon, 21 Oct 2024 at 20:31, Adrian Klaver <adrian.klaver@aklaver.com>
wrote:

Hello,
Sorry for nagging, but I would really like to find some answers.
So, to reiterate. Experiment done as follows:
"""""""""""""""""""""'
CREATE TABLE refs (
id integer primary key,
did integer
);
CREATE TABLE films (
id integer,
code char(5) ,
title varchar(40) NOT NULL,
did integer NOT NULL references refs(id)
)
partition by list (code);

insert into refs values (5, 5)
create table films_partition (LIKE films INCLUDING ALL)
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code = 'dr');
alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);
"""""""""""""""""""""""""
Then, when we open a transaction and try to attach:
"""""""""""""""""""""""""
BEGIN;
ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')
keep the transaction running..
""""""""""""""""""""""""
Once we check a locks, we will see that there is AccessExclusiveLock on
table refs.
""""""
select relname, mode
from pg_locks l
join pg_class c on (relation = c.oid)
join pg_namespace nsp on (c.relnamespace = nsp.oid);
"""""
My questions are:
1. Why is postgres adding again a constraint? Can't it detect that foreign
key already exists? I want to avoid locking partitioned table for too long.
2. Even when attach is adding a foreign key again, why is there
AccessExclusiveLock on refs table? foreign key constraint addition does not
require it.
https://pglocks.org/?pgcommand=ALTER%20TABLE%20ADD%20FOREIGN%20KEY%20(CHILD)
3. If I repeat the steps listed above, but do not add foreign key manually,
then attach partition does not hold AccessExclusive lock on table refs. It
still needs to add a foreign key, as "films" table has that constraint. Why
is the AccessExclusive lock missing from "refs" table now?

Regards!

---------- Forwarded message ---------
From: user <user@pidu.dev>
Date: Fri, 1 Nov 2024 at 15:35
Subject: Re: Fwd: Postgres attach partition: AccessExclusive lock set on
different tables depending on how attaching is performed
To: <adrian.klaver@aklaver.com>
Cc: <pgsql-general@lists.postgresql.org>

Hello Adrian,
My apology for answering so late.

Indeed I have not splitted my transactions correctly in my first example.
BUT, if you change the syntaxt so that attach is performed in its own
transaction, result is the same. Try it out!

Also you have told me that I am seeing ExclusiveLock on refs table because
it is from foreign key constraint. Well this is incorrect, foreign key does
not take the ExclusiveLock in any situation.
https://pglocks.org/?pgcommand=ALTER%20TABLE%20ADD%20FOREIGN%20KEY%20(CHILD)

Could you try again with only attach being in its own transaction? You
should reproduce it.
Regards

On Mon, 21 Oct 2024 at 20:31, Adrian Klaver <adrian.klaver@aklaver.com>
wrote:

On 11/10/24 05:18, user wrote:

Hello,
Sorry for nagging, but I would really like to find some answers.
So, to reiterate. Experiment done as follows:
"""""""""""""""""""""'
CREATE TABLE refs (
                    id integer primary key,
                    did integer
                );
CREATE TABLE films (
                    id integer,
                    code char(5) ,
                    title varchar(40) NOT NULL,
                    did integer NOT NULL references refs(id)
                    )
                    partition by list (code);

insert into refs values (5, 5)
create table films_partition (LIKE films INCLUDING ALL)
insert into films_partition values (1, 'dr', 'musician',5)
alter table films_partition add constraint check_code check (code = 'dr');
alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);
"""""""""""""""""""""""""
Then, when we open a transaction and try to attach:
"""""""""""""""""""""""""
 BEGIN;
ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')
keep the transaction running..
""""""""""""""""""""""""
Once we check a locks, we will see that there is AccessExclusiveLock on
table refs.
""""""
select relname, mode
                from pg_locks l
                    join pg_class c on (relation = c.oid)
                    join pg_namespace nsp on (c.relnamespace = nsp.oid);
"""""
My questions are:
1. Why is postgres adding again a constraint? Can't it detect that
foreign key already exists? I want to avoid locking partitioned table
for too long.

I see, I missed it my previous post:

alter table films_partition ADD CONSTRAINT fk_did FOREIGN KEY (did)
REFERENCES refs (id);

\d films_partition

Foreign-key constraints:
"fk_did" FOREIGN KEY (did) REFERENCES refs(id)

ALTER TABLE films ATTACH PARTITION films_partition for values in ('dr')

\d films_partition

Foreign-key constraints:
TABLE "films" CONSTRAINT "films_did_fkey" FOREIGN KEY (did)
REFERENCES refs(id)

The FK constraint changes from being

films_partition <--> refs

to

films <--> refs

2. Even when attach is adding a foreign key again, why is there
AccessExclusiveLock on refs table? foreign key constraint addition does
not require it.
https://pglocks.org/?pgcommand=ALTER%20TABLE%20ADD%20FOREIGN%20KEY%20(CHILD) <https://pglocks.org/?pgcommand=ALTER%20TABLE%20ADD%20FOREIGN%20KEY%20(CHILD)&gt;
3. If I repeat the steps listed above, but do not add foreign key
manually, then attach partition does not hold AccessExclusive lock on
table refs. It still needs to add a foreign key, as "films" table has
that constraint. Why is the AccessExclusive lock missing from "refs"
table now?

Best guess because the FK is changing referencing table and in:

~//src/backend/commands/tablecmds.c

"CloneFkReferencing

For each FK constraint of the parent relation in the given list, find an
equivalent constraint in its partition relation that can be reparented;
if one cannot be found, create a new constraint in the partition as its
child."

[...]

addFkRecurseReferencing(wqueue,
fkconstraint,
partRel,
pkrel,
indexOid,
constrOid,
numfks,
confkey,
mapped_conkey,
conpfeqop,
conppeqop,
conffeqop,
numfkdelsetcols,
confdelsetcols,
false, /* no old check exists */
AccessExclusiveLock,
insertTriggerOid,
updateTriggerOid);

Regards!

--
Adrian Klaver
adrian.klaver@aklaver.com

Thank you for an answer!
So reparenting of a constraint required that additional lock.
I made some measurements and can see that even that reparenting (and
additional lock) is required, the time it takes to make the attach is
smaller than when the foreign constraint hasn't been created beforehand.

So, to summarise, there is a tradeoff.
1. Create constraint before attach, but during attach additional tables
will be locked with AccessExculive. The time of an attach will be minimal
(for large tables it is still tens of ms in our db) but there is a higher
chance of deadlocks (as more tables locked with restrictive locks)
2. Just proceed with attach. The constraint will be created because the
parent table has the constraint in its definition. Because no reparenting
is required, no additional exclusive lock is held. But this process will
take more time to finish as a constraint is created from scratch.

Are these the only options?
Basically I want to add partitions dynamically to db while app is running.
I want to minimise the duration of "attach" command but also the amount of
locks held on several tables at once (to avoid deadlocks).

Once again, thanks for an answer. It is now clear to me why such behaviour
occurs.
Regards

On Sun, 10 Nov 2024, 20:07 Adrian Klaver, <adrian.klaver@aklaver.com> wrote:

On 11/10/24 11:52, user wrote:

Thank you for an answer!
So reparenting of a constraint required that additional lock.
I made some measurements and can see that even that reparenting (and
additional lock) is required, the time it takes to make the attach is
smaller than when the foreign constraint hasn't been created beforehand.

So, to summarise, there is a tradeoff.
1. Create constraint before attach, but during attach additional tables
will be locked with AccessExculive. The time of an attach will be
minimal  (for large tables it is still tens of ms in our db) but there
is a higher chance of deadlocks (as more tables locked with restrictive
locks)
2. Just proceed with attach. The constraint will be created because the
parent table has the constraint in its definition. Because no
reparenting is required, no additional exclusive lock is held. But this
process will take more time to finish as a constraint is created from
scratch.

Are these the only options?
Basically I want to add partitions dynamically to db while app is
running. I want to minimise the duration of "attach" command but also
the amount of locks held on several tables at once (to avoid deadlocks).

Once again, thanks for an answer. It is now clear to me why such
behaviour occurs.

Just to be clear:

1) I had nothing to do with writing this code.

2) I am not a C programmer, so what you got was my creative
interpretation of what I think is going on.

3) Because of 1 & 2, this needs further analysis by someone or someones
more knowledgeable.

Regards

--
Adrian Klaver
adrian.klaver@aklaver.com

Adrian Klaver <adrian.klaver@aklaver.com> writes:

3) Because of 1 & 2, this needs further analysis by someone or someones
more knowledgeable.

This surprised me a bit too, because I thought we took a
slightly-less-than-exclusive lock for FK additions or deletions.
Tracing through it, I find that CloneFkReferencing opens the
referenced relation with ShareRowExclusiveLock as I expected.
But then we conclude that we can drop the existing FK enforcement
triggers for the table being attached. That causes us to take
AccessExclusiveLock on the trigger itself, which is fine because
nobody's really paying attention to that. But then RemoveTriggerById
takes AccessExclusiveLock on the trigger's table. We already had
that on the table being attached, but not on the other table.

This is quite bad, not just because the AccessExclusiveLock might
block other stuff but because we are doing a lock upgrade on the
referenced table, greatly increasing the risk of deadlock.

I wonder whether it'd be all right for RemoveTriggerById to take
only ShareRowExclusiveLock on the trigger's table. This seems
OK in terms of basic semantics: that's enough to lock out
anything that might want to fire triggers on the table. However,
this comment for AlterTableGetLockLevel gives me pause:

* Also note that pg_dump uses only an AccessShareLock, meaning that anything
* that takes a lock less than AccessExclusiveLock can change object definitions
* while pg_dump is running. Be careful to check that the appropriate data is
* derived by pg_dump using an MVCC snapshot, rather than syscache lookups,
* otherwise we might end up with an inconsistent dump that can't restore.

I think pg_dump uses pg_get_triggerdef, which is probably not
safe in these terms.

An alternative answer might be what Alvaro was muttering about
the other day: redesign FKs for partitioned tables so that we
do not have to change the set of triggers when attaching/detaching.

regards, tom lane

On 2024-Nov-10, Tom Lane wrote:

This surprised me a bit too, because I thought we took a
slightly-less-than-exclusive lock for FK additions or deletions.
Tracing through it, I find that CloneFkReferencing opens the
referenced relation with ShareRowExclusiveLock as I expected.
But then we conclude that we can drop the existing FK enforcement
triggers for the table being attached. That causes us to take
AccessExclusiveLock on the trigger itself, which is fine because
nobody's really paying attention to that. But then RemoveTriggerById
takes AccessExclusiveLock on the trigger's table. We already had
that on the table being attached, but not on the other table.

Oooh.

I wonder whether it'd be all right for RemoveTriggerById to take
only ShareRowExclusiveLock on the trigger's table. This seems
OK in terms of basic semantics: that's enough to lock out
anything that might want to fire triggers on the table. However,
this comment for AlterTableGetLockLevel gives me pause:

* Also note that pg_dump uses only an AccessShareLock, meaning that anything
* that takes a lock less than AccessExclusiveLock can change object definitions
* while pg_dump is running. Be careful to check that the appropriate data is
* derived by pg_dump using an MVCC snapshot, rather than syscache lookups,
* otherwise we might end up with an inconsistent dump that can't restore.

I think pg_dump uses pg_get_triggerdef, which is probably not
safe in these terms.

Looking at pg_get_triggerdef_worker, it is not using syscache but a
systable scan, which uses the catalog snapshot. A catalog snapshot is
indeed implemented as an MVCC snapshot (so strictly speaking it _is_ an
MVCC snapshot), but the invalidation rules are different from a normal
MVCC snapshot, so AFAIU it's still unsafe.

An alternative answer might be what Alvaro was muttering about
the other day: redesign FKs for partitioned tables so that we
do not have to change the set of triggers when attaching/detaching.

Hmm, I hadn't thought about this idea in those terms, but perhaps we
could reimplement this by not having one trigger for each RI check, but
instead a single trigger which internally determines which FK
constraints exist on the table and does the necessary work in a single
pass. Then we don't need to add/drop triggers all the time, but we just
add it with the first FK in the table, and remove it when dropping the
last FK.

For tables with many FKs, this could be a win, because we'd only go
through the trigger machinery once. If a table has both outgoing and
incoming FKs, maybe we could have _one_ single trigger.

(I think this would be orthogonal with the project to stop using SPI for
RI triggers.)

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/