Skip to content

Conversation

@ZTE-EBASE
Copy link

Fixes #ENUM_HASH_PARTITION_ISSUE
Fixes #PARALLEL_RESTORE_DEADLOCK

What does this PR do?

  1. Automatic partition handling: Auto-enables --load-via-partition-root when dumping partitioned tables with hash partitioning on enum types to prevent partition constraint violations after dump/reload cycles
  2. Parallel restore safety: Modifies pg_restore to skip TRUNCATE operations when partition root loading is detected, eliminating deadlocks and data loss risks
  3. Dump metadata enhancement: Adds special comment markers for TABLE DATA items using partition root loading mode
  4. Documentation update: Removes obsolete warning about --load-via-partition-root with parallel restore from pg_dump documentation

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)

Breaking Changes

None

Test Plan

Impact

Performance:

User-facing changes:

No significant change in dump performance for partitioned tables
Parallel restore stability improves overall throughput by eliminating deadlock points

Dependencies:

Users no longer need to manually specify --load-via-partition-root for enum hash partitions
Eliminates data corruption risks during pg_upgrade of partitioned tables
Provides clearer dump file annotation structure

Checklist

Additional Context

Compatibility note​​:
For existing dumps using --inserts + --load-via-partition-root:

Parallel restore might still trigger deadlocks (legacy file limitation)
Mitigation: Use single-threaded restore or regenerate dump files

CI Skip Instructions


@my-ship-it my-ship-it requested a review from yjhjstz June 20, 2025 02:54
@ZTE-EBASE ZTE-EBASE closed this Jul 1, 2025
@ZTE-EBASE ZTE-EBASE deleted the fix_pg_dump branch July 1, 2025 01:19
@ZTE-EBASE ZTE-EBASE restored the fix_pg_dump branch July 1, 2025 01:40
@ZTE-EBASE
Copy link
Author

Automatic partition handling: Auto-enables --load-via-partition-root when dumping partitioned tables with hash partitioning on enum types to prevent partition constraint violations after dump/reload cycles
Parallel restore safety: Modifies pg_restore to skip TRUNCATE operations when partition root loading is detected, eliminating deadlocks and data loss risks
Dump metadata enhancement: Adds special comment markers for TABLE DATA items using partition root loading mode
Documentation update: Removes obsolete warning about --load-via-partition-root with parallel restore from pg_dump documentation

@ZTE-EBASE ZTE-EBASE reopened this Jul 1, 2025
@tuhaihe tuhaihe self-requested a review July 1, 2025 07:26
Copy link
Member

@tuhaihe tuhaihe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates:

This PR is cherry-picked from the PG upstream: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=bc8cd50fefd369b217f80078585c486505aafb62#patch4.

When cherry-picking commits, we need to preserve the original commit and message. If changes are necessary, create a new commit to apply them, thereby respecting the original author's contributions, as described here.

I would like to request changes to this PR again.

@ZTE-EBASE ZTE-EBASE closed this Jul 29, 2025
@ZTE-EBASE ZTE-EBASE reopened this Jul 29, 2025
@tuhaihe tuhaihe force-pushed the fix_pg_dump branch 2 times, most recently from 9a71425 to 515c029 Compare July 30, 2025 10:38
@tuhaihe tuhaihe self-requested a review July 31, 2025 02:33
Copy link
Member

@yjhjstz yjhjstz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tglsfdc and others added 2 commits August 5, 2025 14:11
Hash partitioning on an enum is problematic because the hash codes are
derived from the OIDs assigned to the enum values, which will almost
certainly be different after a dump-and-reload than they were before.
This means that some rows probably end up in different partitions than
before, causing restore to fail because of partition constraint
violations.  (pg_upgrade dodges this problem by using hacks to force
the enum values to keep the same OIDs, but that's not possible nor
desirable for pg_dump.)

Users can work around that by specifying --load-via-partition-root,
but since that's a dump-time not restore-time decision, one might
find out the need for it far too late.  Instead, teach pg_dump to
apply that option automatically when dealing with a partitioned
table that has hash-on-enum partitioning.

Also deal with a pre-existing issue for --load-via-partition-root
mode: in a parallel restore, we try to TRUNCATE target tables just
before loading them, in order to enable some backend optimizations.
This is bad when using --load-via-partition-root because (a) we're
likely to suffer deadlocks from restore jobs trying to restore rows
into other partitions than they came from, and (b) if we miss getting
a deadlock we might still lose data due to a TRUNCATE removing rows
from some already-completed restore job.

The fix for this is conceptually simple: just don't TRUNCATE if we're
dealing with a --load-via-partition-root case.  The tricky bit is for
pg_restore to identify those cases.  In dumps using COPY commands we
can inspect each COPY command to see if it targets the nominal target
table or some ancestor.  However, in dumps using INSERT commands it's
pretty impractical to examine the INSERTs in advance.  To provide a
solution for that going forward, modify pg_dump to mark TABLE DATA
items that are using --load-via-partition-root with a comment.
(This change also responds to a complaint from Robert Haas that
the dump output for --load-via-partition-root is pretty confusing.)
pg_restore checks for the special comment as well as checking the
COPY command if present.  This will fail to identify the combination
of --load-via-partition-root and --inserts in pre-existing dump files,
but that should be a pretty rare case in the field.  If it does
happen you will probably get a deadlock failure that you can work
around by not using parallel restore, which is the same as before
this bug fix.

Having done this, there seems no remaining reason for the alarmism
in the pg_dump man page about combining --load-via-partition-root
with parallel restore, so remove that warning.

Patch by me; thanks to Julien Rouhaud for review.  Back-patch to
v11 where hash partitioning was introduced.

Discussion: https://postgr.es/m/[email protected]
Since we cherry-picked the Postgres pg_dump commit to Cloudberry, we
need to update the files to support Cloudberry CI to let me pass.
@tuhaihe tuhaihe merged commit ccd790e into apache:main Aug 5, 2025
51 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants