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

From: Adrian Klaver <[email-masked]>
Date: Mon, 21 Oct 2024 11:31:10 -0700

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, <[email-masked] 
> <mailto:[email-masked]>> 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>
>      > <https://www.postgresql.org/docs/current/ddl-partitioning.html
>     <https://www.postgresql.org/docs/current/ddl-partitioning.html>>
>     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>
> 
>     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>
> 
>     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
>     [email-masked] <mailto:[email-masked]>
> 

-- 
Adrian Klaver
[email-masked]

---

From: user <[email-masked]>
Date: Fri, 1 Nov 2024 15:35:12 +0100

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

---

From: user <[email-masked]>
Date: Sun, 10 Nov 2024 14:18:16 +0100

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 <[email-masked]>
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: <[email-masked]>
Cc: <[email-masked]>

Hello Adrian,

---

From: Adrian Klaver <[email-masked]>
Date: Sun, 10 Nov 2024 11:07:22 -0800

On 11/10/24 05:18, user wrote:
> 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:

\d films_partition

Foreign-key constraints:

\d films_partition

Foreign-key constraints:

The FK constraint changes from being

films_partition <--> refs

to

films <--> refs

> 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)>
> 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);

---

From: user <[email-masked]>
Date: Sun, 10 Nov 2024 20:52:52 +0100

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, <[email-masked]> wrote:

> <

---

From: Adrian Klaver <[email-masked]>
Date: Sun, 10 Nov 2024 14:16:46 -0800

On 11/10/24 11:52, user wrote:
> 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)
> 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.
> 
> 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.

---

From: Tom Lane <[email-masked]>
Date: Sun, 10 Nov 2024 18:44:39 -0500

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

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