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
From d91ccacc29b80dd502cd620f8d62bfe02e24d94b Mon Sep 17 00:00:00 2001 From: Rajesh Kumar Thandapani <[email protected]> Date: Sun, 1 Nov 2026 07:27:49 +0000 Subject: [PATCH v2] Fix pg_combinebackup with truncated intermediate backups. When reconstructing a relation from a chain of backups, reconstruct_from_incremental_file() walks backward from the latest incremental backup to the full backup, filling sourcemap[] for any block b < latest_source->truncation_block_length that has not yet been assigned a source. However, if an intermediate incremental backup truncated the relation (s->truncation_block_length < latest_source->truncation_block_length) and a later backup extended the relation again without writing every block in the truncated range (for example, via bulk relation extension), those unwritten blocks still had sourcemap[b] == NULL when the loop moved past the intermediate backup. As a result, they were sourced from an earlier backup taken before the truncation, resurrecting stale rows. Fix by tracking the current truncation_block_length upper bound as we walk backward through the backup chain and tightening it whenever an incremental backup has a smaller s->truncation_block_length. Author: Rajesh Kumar Thandapani <[email protected]> Bug: #19776 Reported-by: Ke <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/bin/pg_combinebackup/reconstruct.c | 17 +++++- .../pg_combinebackup/t/011_ib_truncation.pl | 59 ++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c index 5900c5b..5d875cc 100644 --- a/src/bin/pg_combinebackup/reconstruct.c +++ b/src/bin/pg_combinebackup/reconstruct.c @@ -105,6 +105,7 @@ reconstruct_from_incremental_file(char *input_filename, rfile **sourcemap; off_t *offsetmap; unsigned block_length; + unsigned truncation_block_length; unsigned sidx = n_prior_backups; bool full_copy_possible = true; int copy_source_index = -1; @@ -128,6 +129,7 @@ reconstruct_from_incremental_file(char *input_filename, latest_source = make_incremental_rfile(input_filename); source[n_prior_backups] = latest_source; block_length = find_reconstructed_block_length(latest_source); + truncation_block_length = latest_source->truncation_block_length; /* * For each block in the output file, we need to know from which file we @@ -224,7 +226,7 @@ reconstruct_from_incremental_file(char *input_filename, * later backup is one that we can't validate. */ blocklength = sb.st_size / BLCKSZ; - for (b = 0; b < latest_source->truncation_block_length; ++b) + for (b = 0; b < truncation_block_length; ++b) { if (sourcemap[b] == NULL && b < blocklength) { @@ -264,7 +266,7 @@ reconstruct_from_incremental_file(char *input_filename, { BlockNumber b = s->relative_block_numbers[i]; - if (b < latest_source->truncation_block_length && + if (b < truncation_block_length && sourcemap[b] == NULL) { sourcemap[b] = s; @@ -278,6 +280,17 @@ reconstruct_from_incremental_file(char *input_filename, full_copy_possible = false; } } + + /* + * Any blocks at or beyond this incremental file's + * truncation_block_length did not exist at the time of this backup, + * so we must not source them from any earlier backup. + */ + if (s->truncation_block_length < truncation_block_length) + { + truncation_block_length = s->truncation_block_length; + full_copy_possible = false; + } } /* diff --git a/src/bin/pg_combinebackup/t/011_ib_truncation.pl b/src/bin/pg_combinebackup/t/011_ib_truncation.pl index 1174928..d6d6427 100644 --- a/src/bin/pg_combinebackup/t/011_ib_truncation.pl +++ b/src/bin/pg_combinebackup/t/011_ib_truncation.pl @@ -115,7 +115,64 @@ my $restored_count = is($restored_count, $rows_after_truncation, 'Restored backup has correct row count'); -$primary->stop; $restored->stop; +# Test that blocks truncated in an intermediate backup are not resurrected +# when a later backup bulk-extends the relation with unwritten zero-filled +# pages. Increase shared_buffers so COPY can bulk-extend by multiple blocks +# at once, grow the table to 30 blocks, truncate it in an intermediate +# backup, and then bulk-extend it with COPY. +$primary->append_conf('postgresql.conf', 'shared_buffers = 8MB'); +$primary->restart; + +my $full_rows = 30; +$primary->safe_psql( + 'postgres', + "INSERT INTO t + SELECT i, + repeat('0123456789ABCDEF0123456789ABCDEF', ($block_size / (2*32))) + FROM generate_series($rows_after_truncation + 1, $full_rows) i; + VACUUM t;" +); + +my $incr_backup = "$backup_path/incr"; +$primary->backup('incr2', + backup_options => [ '--incremental', "$incr_backup/backup_manifest" ]); + +$primary->safe_psql('postgres', + "DELETE FROM t WHERE id > $rows_after_truncation; + VACUUM (TRUNCATE) t;"); + +my $incr2_backup = "$backup_path/incr2"; +$primary->backup('incr3', + backup_options => [ '--incremental', "$incr2_backup/backup_manifest" ]); + +my $copy_rows = 17; +my $row_data = '0123456789ABCDEF0123456789ABCDEF' x ($block_size / (2 * 32)); +my $copy_input = "COPY t FROM STDIN;\n"; +for my $i (1 .. $copy_rows) +{ + $copy_input .= ($full_rows + $i) . "\t$row_data\n"; +} +$copy_input .= "\\.\n"; +$primary->safe_psql('postgres', $copy_input); + +my $incr3_backup = "$backup_path/incr3"; +$primary->backup('incr4', + backup_options => [ '--incremental', "$incr3_backup/backup_manifest" ]); + +my $restored2 = PostgreSQL::Test::Cluster->new('node3'); +$restored2->init_from_backup( + $primary, 'incr4', + combine_with_prior => [ 'full', 'incr', 'incr2', 'incr3' ]); +$restored2->start(); + +$restored_count = + $restored2->safe_psql('postgres', "SELECT count(*) FROM t;"); +is($restored_count, $rows_after_truncation + $copy_rows, + 'Restored backup after intermediate truncation and bulk extension has correct row count'); + +$primary->stop; +$restored2->stop; + done_testing(); -- 2.56.0.385.gac671c819a-goog
