Repository navigation
Add incremental support for pax and fix incremental for ao tables - #136
Merged
Merged
Conversation
tuhaihe
reviewed
Nov 6, 2026
An incremental backup re-read every PAX table unconditionally. PAX has no
pg_aoseg relation and therefore no modcount, so FilterTablesForIncremental
found no change markers for it and fell through to the "always include"
branch meant for heap and external tables. Correct, but it makes an
incremental backup of a PAX database cost the same as a full one.
What PAX does have is an aux relation in pg_ext_aux describing its
micro-partitions, and Cloudberry builds it in one of two mutually
exclusive layouts fixed at compile time:
manifest (USE_MANIFEST_API) pg_ext_aux.pg_manifest_<oid>, one row per
segment whose path names the current
manifest file; any transaction that
dirties the manifest flushes it under a
freshly generated name
catalog (USE_PAX_CATALOG) pg_ext_aux.pg_pax_blocks_<oid>, one row
per micro-partition; inserts add rows,
deletes write a new visimap file and
update ptvisimapname
Detect the layout by aux relation name rather than by version, since
nothing in the catalog says which way the cluster was built, and reduce
either one to a single fixed-width digest.
The digest is SHA-256, not MD5. The dangerous direction here is two
different aux states hashing alike, because that makes a changed table
look unchanged and silently drops its data from the backup. MD5 has
constructible collisions, so it is the wrong primitive for a check whose
failure mode is data loss, and the hash is far too cheap for its strength
to be worth trading away.
Hashing at all, rather than keeping the aux rows, is what keeps the
marker bounded: one is held in memory and written to the TOC for every
PAX table in the backup set, so it has to be a fixed width independent of
how many micro-partitions a table has. For the catalog layout the rows
are digested per segment on that segment and only the per-segment digests
are combined on the coordinator, so coordinator memory is proportional to
the number of segments rather than to the size of the table.
Key the digest by gp_segment_id. Block numbers are assigned per segment,
so the same ptblockname occurs once per segment with unrelated contents;
folding the segments together would let two different cluster states
reduce to the same multiset of rows.
Pair the marker with the same last CREATE/ALTER/TRUNCATE timestamp the AO
path already uses, so a PAX table is re-read when either signal differs.
A partitioned root has no aux relation of its own and so is absent from
the marker map, exactly as an AO root is absent from the modcount map.
Like the AO modcount this over-reports rather than under-reports: a
rewrite that preserves the data changes the digest and causes an
unnecessary re-backup. A read-only transaction does not dirty the aux
relation, so plain queries never trigger one.
The end_to_end fixture covers heap plain, heap partitioned, ao_row plain
and ao_column plain. Nothing covers pax, nothing covers an AO or PAX
partitioned table, and nothing covers a partitioned table whose leaves
do not all share one access method. Incremental backup is exactly where
the access methods stop behaving alike, so that is a gap worth closing.
Add end_to_end/storage_matrix_test.go, which builds every access method
Cloudberry can store user data in, crossed with the three shapes a table
can take:
plain uniform-AM partitioned leaf of a mixed-AM
partitioned table
heap x x x
ao_row x x x
ao_column x x x
pax x x x
The mixed-AM table is the shape that is not otherwise reachable: its
leaves each use a different access method and its root carries none.
Classic Greenplum partition syntax cannot express it, because
per-partition WITH (appendonly=...) has no spelling for pax, so it is
built with declarative partitioning while the uniform tables keep the
classic syntax and both paths get exercised.
Five specs run over that schema:
- a full backup and restore, compared by data fingerprint
- a full backup and restore, compared by per-relation access method
- an incremental backup, asserting which tables land in which restore
plan entry
- a plain restore of the incremental timestamp, which walks the chain
- gprestore --incremental --data-only applied onto a database restored
from the full backup
The incremental specs change exactly one table per access method and
assert restore plan membership, because that is what differs: heap has
no change detection and is re-read every time, while AO and PAX tables
stay with the backup that last read them. The PAX change is a delete,
which PAX records in the visimap without adding a micro-partition, so it
is the shape a marker derived from the micro-partition sequence alone
would miss.
The matrix is built from the access methods the cluster actually has, so
it degrades to heap/ao_row/ao_column where the pax contrib module is
absent rather than skipping wholesale. It needs USING <access method>,
so it skips below GPDB7.
Two findings worth recording, both pre-existing:
Data fingerprints are a sum of per-row digests, not an ordered
string_agg. The obvious md5(string_agg(v, ',' ORDER BY id)) is unusable
in MPP: segments produce partial aggregates that are concatenated in
arrival order, so the same query over the same data returned three
different values in three consecutive runs.
A partitioned root explicitly created USING heap loses its access method
across backup and restore. GetTableAccessMethod() drops amname = 'heap'
because heap is the default for an ordinary table, but the default for a
partitioned root is no access method at all, so nothing is emitted and
the restored root has none. Leaves and data are unaffected; what changes
is the access method a later ADD PARTITION inherits. The spec asserts
the current behavior rather than ignoring it, so closing the gap will be
noticed here.
Add a storage_matrix make target to run just these specs.
Tested on two demo clusters, one built with each PAX aux layout. Run
together with the existing incremental specs to check for interference:
32 passed, and the 2 failures are the "With a plugin" specs, which fail
the same way on an unmodified tree in an environment with no backup
plugin configured.
An incremental backup silently skipped a table that CLUSTER had
rewritten. The restore plan kept pointing at the copy taken before the
rewrite, so restoring the incremental handed back stale contents. The
backup reported success; nothing in the logs distinguished it from a
table that genuinely had not changed.
gpbackup decided whether an AO table changed from two signals: the sum
of modcount over its pg_aoseg relation, and the last
CREATE/ALTER/TRUNCATE timestamp. A rewrite defeats both at once.
cluster.c builds a transient relation with make_new_heap(), which gets
its own pg_aoseg, copies the live rows in, and swaps. The table ends up
on a different pg_aoseg whose rows were written by exactly one
statement, so the sum restarts at the number of primary segments --
which is also what a freshly bulk-loaded table reads:
after load sum(modcount) = 3 100000 rows
after delete sum(modcount) = 6 70000 rows
after CLUSTER sum(modcount) = 3 70000 rows
and on a two-segment cluster, 2 -> 4 -> 2. Meanwhile CLUSTER is not one
of the actions pg_stat_last_operation records, so the DDL timestamp does
not move either.
The rewrite itself changes no rows. What it destroys is the evidence of
the DML that came before it, so the data actually lost is the earlier
DELETE. The sequence that hits this is ordinary maintenance: load a
table, back it up, run a period of updates, CLUSTER to restore locality,
back up again.
Enumerating what rewrites an AO table turns up two more operations with
the same shape, both of which reset the counter without being recorded:
CLUSTER modcount 6 -> 3, no DDL stamp
SET UNLOGGED modcount 6 -> 3, no DDL stamp
SET ACCESS METHOD modcount 6 -> 3, no DDL stamp
Every other rewrite -- SET WITH (reorganize=true), SET DISTRIBUTED BY,
ALTER COLUMN TYPE -- also resets the counter but is spelled ALTER in a
form the catalog records, so the timestamp catches it.
Record a third signal: the table's relfilenode, read from the
coordinator and carried as int64, since a relfilenode is an oid and oid
casts to bigint by assignment on every supported version. Any one of the
three signals differing re-reads the table. They cover each other: the
content signal (AO modcount, PAX aux digest) misses pure DDL, the DDL
timestamp misses the three rewrites above, and the storage identity
misses nothing about a rewrite but says nothing about DML.
The coordinator's relfilenode is not the value the segments hold -- the
two are allocated independently and never match, a table reading 28848
on the coordinator reads 60115 on every segment -- but they move
together, because a rewrite is a dispatched DDL that replaces the
relation on every node inside one distributed transaction. Measured
across all seven rewrites above on both ao_row and ao_column, the two
sides changed together every time, and VACUUM FULL left both alone:
初始 base_rf=0:56106,1:56106,2:56106 aoseg_oid=61781
DELETE base_rf=0:56106,1:56106,2:56106 aoseg_oid=61781
VACUUM FULL base_rf=0:56106,1:56106,2:56106 aoseg_oid=61781
CLUSTER base_rf=0:56120,1:56120,2:56120 aoseg_oid=61803
Reading the coordinator rather than gathering from the segments keeps
this to one local catalog query, and avoids depending on an ordered
aggregate surviving a motion. It also means adding segments does not
invalidate every marker: gpexpand leaves the coordinator's relfilenode
alone, and expansion does not change what a logical backup dumps.
The same signal closes a latent gap in the PAX catalog layout, whose
digest of (ptblockname, pttupcount, ptblocksize, ptvisimapname) can in
principle reproduce an earlier value after a rewrite resets block
numbering and clears the visimap. It survived the reproduction only
because the rewritten micro-partitions happened to differ in tuple
count.
A TOC written before this field existed reads it back as zero, and the
comparison treats "same as last time" as unchanged, so zero has to be
unambiguous. It is: the only relations carrying relfilenode 0 are
partitioned roots, and a root has neither a pg_appendonly entry nor a
PAX aux relation, so it never reaches these maps in the first place.
relname | relkind | amname | relfilenode
-------------+---------+--------+-------------
aopart | p | ao_row | 0
aopart_..._1| r | ao_row | 56517
paxpart | p | pax | 0
paxpart_.._1| r | pax | 56529
Zero therefore means "not recorded", and the first incremental backup
after upgrading re-reads every AO and PAX table once -- the safe
direction, and the only correct answer for a backup whose storage
identity was never recorded.
The fix costs nothing in extra re-reads otherwise: the relfilenode is
left alone by INSERT, UPDATE, DELETE, VACUUM and VACUUM FULL.
Add end_to_end/rewrite_detection_test.go, which runs the sequence per
access method and per rewrite and asserts both the proximate symptom
(the table must appear in the incremental's restore plan entry) and the
actual harm (the restored table must hold 70000 rows, not 100000). That
matrix is also what pins down the assumption this rests on, that a
rewrite reaches the coordinator. PAX skips the SET ACCESS METHOD case:
the engine rejects the conversion with "PAX not allow swap relation
files for different AM". heap is not covered at all, deliberately -- it
has no change detection and is re-read on every incremental, so a
rewrite cannot be missed for it.
Also correct the comment above getModCount. It warned that VACUUM FULL
produces a false positive; measured, VACUUM FULL changes neither the
modcount nor the aoseg relation, and the real hazard is the opposite one
this commit fixes.
Tested on two demo clusters, one per PAX aux layout: 8 rewrite specs
pass on both, and the TOC round trips.
gfphoenix78
force-pushed
the
fix-pax-support
branch
from
November 6, 2026 09:48
442bfef to
f2df194
Compare
Contributor
|
Thanks — the approach is sound and errs toward re-reading. I couldn't find a path where a changed table is skipped. The relfilenode signal also fixes a real AO problem: a DELETE followed by CLUSTER was previously missed by the incremental. |
gfphoenix78
changed the title
gfphoenix78
enabled auto-merge (rebase)
November 6, 2026 10:21
my-ship-it
approved these changes
Nov 6, 2026
Member
|
Hi @gfphoenix78, we need to make sure all the tests pass. There are some failed tests, please create another PR on them. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add incremental support for pax and fix incremental backup for ao/co table
Change logs
Contributor's checklist
Here are some reminders before you submit your pull request: