Subject: Re: pg_dump versus hash partitioning

From: Tom Lane <[email-masked]>
Date: Fri, 10 Mar 2023 22:10:14 -0500

Julien Rouhaud <[email-masked]> writes:
> Working on some side project that can cause dump of hash partitions to be
> routed to a different partition, I realized that --load-via-partition-root can
> indeed cause deadlock in such case without FK dependency or anything else.

> The problem is that each worker will perform a TRUNCATE TABLE ONLY followed by
> a copy of the original partition's data in a transaction, and that obviously
> will lead to deadlock if the original and locked partition and the restored
> partition are different.

Oh, interesting.  I wonder if we can rearrange things to avoid that.

			regards, tom lane

---

From: Julien Rouhaud <[email-masked]>
Date: Sat, 11 Mar 2023 11:32:32 +0800

> Oh, interesting.  I wonder if we can rearrange things to avoid that.

The BEGIN + TRUNCATE is only there to avoid generating WAL records just in case
the wal_level is minimal.  I don't remember if that optimization still exists,
but if yes we could avoid doing that if the server's wal_level is replica or
higher?  That's not perfect but it would help in many cases.

---

From: Tom Lane <[email-masked]>
Date: Sun, 12 Mar 2023 15:46:52 -0400

> higher?  That's not perfect but it would help in many cases.

After thinking about this, it seems like a better idea is to skip the
TRUNCATE if we're doing load-via-partition-root.  In that case it's
clearly a dangerous thing to do regardless of deadlock worries, since
it risks discarding previously-loaded data that came over from another
partition.  (IOW this seems like an independent, pre-existing bug in
load-via-partition-root mode.)

The trick is to detect in pg_restore whether pg_dump chose to do
load-via-partition-root.  If we have a COPY statement we can fairly
easily examine it to see if the target table is what we expect or
something else.  However, if the table was dumped as INSERT statements
it'd be far messier; the INSERTs are not readily accessible from the
code that needs to make the decision.

What I propose we do about that is further tweak things so that
load-via-partition-root forces dumping via COPY.  AFAIK the only
compelling use-case for dump-as-INSERTs is in transferring data
to a non-Postgres database, which is a context in which dumping
partitioned tables as such is pretty hopeless anyway.  (I wonder if
we should have some way to dump all the contents of a partitioned
table as if it were unpartitioned, to support such migration.)

An alternative could be to extend the archive TOC format to record
directly whether a given TABLE DATA object loads data via partition
root or normally.  Potentially we could do that without an archive
format break by defining te->defn for TABLE DATA to be empty for
normal dumps (as it is now) or something like "-- load via partition root"
for the load-via-partition-root case.  However, depending on examination
of the COPY command would already work for the vast majority of existing
archive files, so I feel like it might be the preferable choice.

Thoughts?

			regards, tom lane

---

From: Justin Pryzby <[email-masked]>
Date: Sun, 12 Mar 2023 14:54:50 -0500

> table as if it were unpartitioned, to support such migration.)

I think that what this other thread is about.

https://commitfest.postgresql.org/42/4130/
pg_dump all child tables with the root table

---

From: Tom Lane <[email-masked]>
Date: Sun, 12 Mar 2023 16:02:43 -0400

> pg_dump all child tables with the root table

As far as I understood (didn't actually read the latest patch) that
one is just about easily selecting all the partitions of a partitioned
table when doing a selective dump.  It's not helping you produce a
non-Postgres-specific dump.

Although I guess by combining load-via-partition-root, data-only mode,
and dump-as-inserts you could produce a clean collection of
non-partition-dependent INSERT commands ... so maybe we'd better not
force dump-as-inserts off.  I'm starting to like the te->defn hack
more.

			regards, tom lane

---

From: Julien Rouhaud <[email-masked]>
Date: Mon, 13 Mar 2023 16:33:14 +0800

> load-via-partition-root mode.)

It's seems quite unlikely to be able to actually truncate already restored data
without eventually going into a deadlock, but it's still possible so agreed.

> partitioned tables as such is pretty hopeless anyway.

It seems acceptable to me.

> (I wonder if

(this would be nice to have)

> archive files, so I feel like it might be the preferable choice.

Given that this approach wouldn't help with existing dump files (at least if
using COPY, in any case the one using INSERT are doomed), so I'm slightly in
favor of the first approach, and later add an easy and non magic incantation
way to produce dumps that don't depend on partitioning.  It would mean that you
would only be able to produce such dumps using pg16 client binaries, but such
version would also work with older server versions so it doesn't seem like a
huge problem in the long run.