Re: BUG #19776: pg_combinebackup ignores the truncation_block_length of an intermediate incremental backup

Started by Rajesh Kumar Thandapani7 days ago3 messageshackersbugs
Jump to latest
#1Rajesh Kumar Thandapani
rajeshkumarth@google.com
bugshackers

Hi Ke,

This problem occurs because in reconstruct_from_incremental_file(),
pg_combinebackup determines which blocks require reconstruction based only on
latest_source->truncation_block_length, without considering the
truncation_block_length of intermediate backups.

Thanks for the report and reproducer.

While walking backward through the backup chain in
reconstruct_from_incremental_file(), we only check
b < latest_source->truncation_block_length when deciding whether an
unassigned block in sourcemap[] can be taken from an earlier backup. If an
intermediate backup truncated the relation and a later backup extended it
again without writing every block in that range (e.g., unwritten pages at
the end of a relation after bulk extension in COPY), those blocks remain
NULL in sourcemap[] and end up being copied from a backup prior to the
truncation.

Attached is a patch that tracks the current truncation_block_length upper
bound as we step backward through the chain and clamps it whenever an
incremental backup has a smaller s->truncation_block_length. Any remaining
unassigned blocks at or above that limit stay NULL in sourcemap[] and are
written as zeroes.

I also extended 011_ib_truncation.pl to cover an intermediate truncation
followed by COPY bulk extension.

Regards,
Rajesh Kumar Thandapani

Attachments:

t254429_1
v1-0001-Fix-pg_combinebackup-with-truncated-intermediate-.patchapplication/x-patch; name=v1-0001-Fix-pg_combinebackup-with-truncated-intermediate-.patchDownload+73-4
#2Rajesh Kumar Thandapani
rajeshkumarth@google.com
In reply to: Rajesh Kumar Thandapani (#1)
bugshackers

Hi all,

Attaching a rebased v2 of the patch so it applies cleanly against
current master for cfbot. No functional changes from v1.

Thanks,
Rajesh

Attachments:

t254429_2
v2-0001-Fix-pg_combinebackup-with-truncated-intermediate-.patchapplication/x-patch; name=v2-0001-Fix-pg_combinebackup-with-truncated-intermediate-.patchDownload+73-4
#3Raja Sai pranav
rajasaipranav0@gmail.com
In reply to: Rajesh Kumar Thandapani (#2)
bugshackers
Re: Re: BUG #19776: pg_combinebackup ignores the truncation_block_length of an intermediate incremental backup

Hi Rajesh,

I reviewed and tested v2.

v2 applies cleanly to master (316861101e8). Linux aarch64, --enable-cassert
--enable-debug --enable-tap-tests, no warnings.

- With master's reconstruct.c, the new 011_ib_truncation test fails 3 of 3
(restored count 25, expected 24). With the patch it passes 3 of 3, and the
whole pg_combinebackup suite passes.

- On a build with --with-segsize-blocks=6 (one of the CI configurations),
the new test passes even without the fix, 3 of 3. There the truncation
empties whole segments: in incr3, 16384.2 to 16384.4 are zero-length full
files, so for the segment that COPY later re-extends (INCREMENTAL.16384.4
in incr4, no blocks) the walk back stops at incr3's empty full file and
never reaches the stale copy. I would expect it to show only when the
truncation point falls inside the segment that is re-extended (I did not
try to build that case). The fix itself is fine there (the suite passes);
only the test can't see the bug in that configuration. Probably acceptable,
but worth knowing.

- The logic looks right to me. The bound only tightens as we walk back, it
is applied after the current file's own blocks are taken (so blocks the
intermediate backup itself re-extended past its truncation_block_length are
still used), and it is only applied to incremental files.

- Nothing tests the full_copy_possible = false in the new block: with that
line removed, the whole suite still passes. I think the line is needed,
though: without it, a chain where no block comes from an incremental file
and the full file happens to be as long as the latest
truncation_block_length would be copied whole, including the blocks the
intermediate backup had truncated away. I couldn't build that case through
the server (regrowing the relation writes blocks that end up in the later
incremental file), so a test may not be practical. A short comment saying
why the line is there would help.

- Small wording nit in the new comment: blocks at or beyond an incremental
file's truncation_block_length are not necessarily blocks that "did not
exist at the time of this backup"; they may have been truncated away at
some point during that backup's WAL range. Something like "Blocks at or
beyond this file's truncation_block_length were truncated away at some
point before this backup, so their contents in any earlier backup are
stale" might be clearer.

Regards,
Sai Pranav

On Mon, Nov 02, 2026 04:32 PM, Rajesh Kumar Thandapani <
rajeshkumarth@google.com> wrote:

Show quoted text

Hi all,

Attaching a rebased v2 of the patch so it applies cleanly against
current master for cfbot. No functional changes from v1.

Thanks,
Rajesh