From c5021cea747df90f3615353821179d56fdf6e147 Mon Sep 17 00:00:00 2001 From: Manu Date: Fri, 23 Oct 2026 22:15:53 -0300 Subject: [PATCH v1 1/2] Refuse truncations from WAL summaries that cross a timeline switch A standby summarizes the WAL it has received, ahead of replay. When it is promoted at a recovery target inside a WAL summary, the summary of the old timeline that covers the target also covers WAL the promoted server never replayed. Block references from that WAL only make an incremental backup copy more than needed, but a relation truncation recorded there was applied too: it discards the references to blocks modified before the switch point, and pg_combinebackup silently takes those blocks from the prior backup. When a summary of a timeline this server switched away from ends after the switch point, refuse a limit block other than 0 from it, asking for a new full backup. Summary entries carry no LSNs, so a truncation before the switch point in the same file is refused too. This goes on top of two fixes for neighboring cases: Ayush Tiwari's, which stops using a summary that starts exactly at the switch point, and Sai Pranav's, which refuses a prior backup whose WAL extends past the switch point. t/014 tests Ayush Tiwari's case; t/015 tests this one. --- src/backend/backup/basebackup_incremental.c | 43 +++++- src/bin/pg_combinebackup/meson.build | 2 + .../t/014_promote_at_summary_start.pl | 106 ++++++++++++++ .../t/015_summary_past_switch.pl | 130 ++++++++++++++++++ 4 files changed, 280 insertions(+), 1 deletion(-) create mode 100644 src/bin/pg_combinebackup/t/014_promote_at_summary_start.pl create mode 100644 src/bin/pg_combinebackup/t/015_summary_past_switch.pl diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 3995f23ad1..fabad28ed2 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -266,7 +266,8 @@ PrepareForIncrementalBackup(IncrementalBackupInfo *ib, MemoryContext oldcontext; List *expectedTLEs; List *all_wslist, - *required_wslist = NIL; + *required_wslist = NIL, + *past_switch_wslist = NIL; ListCell *lc; TimeLineHistoryEntry **tlep; int num_wal_ranges; @@ -568,6 +569,29 @@ PrepareForIncrementalBackup(IncrementalBackupInfo *ib, */ required_wslist = list_concat(required_wslist, tli_wslist); + /* + * A summary that ends after the point where we switched away from + * this timeline also covers WAL that this server may never have + * replayed: a standby summarizes the WAL it has received, ahead of + * replay, and recovery can stop at a recovery target before the end + * of it. Block references from that WAL only make us copy more than + * needed, but a relation truncation recorded there would discard the + * references to blocks modified before the switch. Remember these + * summaries, so that the merge below can refuse such a truncation. + */ + if (tle->tli != backup_state->starttli) + { + ListCell *lc2; + + foreach(lc2, tli_wslist) + { + WalSummaryFile *ws = lfirst(lc2); + + if (ws->end_lsn > tli_end_lsn) + past_switch_wslist = lappend(past_switch_wslist, ws); + } + } + /* * Timelines earlier than the one in which the prior backup began are * not relevant. @@ -587,6 +611,7 @@ PrepareForIncrementalBackup(IncrementalBackupInfo *ib, foreach(lc, required_wslist) { WalSummaryFile *ws = lfirst(lc); + bool past_switch = list_member_ptr(past_switch_wslist, ws); WalSummaryIO wsio; BlockRefTableReader *reader; RelFileLocator rlocator; @@ -605,6 +630,22 @@ PrepareForIncrementalBackup(IncrementalBackupInfo *ib, while (BlockRefTableReaderNextRelation(reader, &rlocator, &forknum, &limit_block)) { + /* + * A limit block of 0 only makes us copy the whole file, which is + * safe. Any other limit from a summary that extends past a + * timeline switch may describe a truncation that never happened + * on this server, and applying it could lose data. + */ + if (past_switch && limit_block != 0 && + limit_block != InvalidBlockNumber) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot use WAL summary file \"%s\" for an incremental backup", + FilePathName(wsio.file)), + errdetail("The file extends past the end of timeline %u and records a relation truncation, which may come from WAL that this server did not replay.", + ws->tli), + errhint("Take a new full backup."))); + BlockRefTableSetLimitBlock(ib->brtab, &rlocator, forknum, limit_block); diff --git a/src/bin/pg_combinebackup/meson.build b/src/bin/pg_combinebackup/meson.build index f1ae1b3594..52412381fc 100644 --- a/src/bin/pg_combinebackup/meson.build +++ b/src/bin/pg_combinebackup/meson.build @@ -41,6 +41,8 @@ tests += { 't/011_ib_truncation.pl', 't/012_vm_consistency.pl', 't/013_diverged_parent.pl', + 't/014_promote_at_summary_start.pl', + 't/015_summary_past_switch.pl', ], } } diff --git a/src/bin/pg_combinebackup/t/014_promote_at_summary_start.pl b/src/bin/pg_combinebackup/t/014_promote_at_summary_start.pl new file mode 100644 index 0000000000..9e150f43d6 --- /dev/null +++ b/src/bin/pg_combinebackup/t/014_promote_at_summary_start.pl @@ -0,0 +1,106 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group +# +# A standby can summarize WAL that it has received but not yet replayed. If +# it is promoted at a recovery target that is exactly the start of such a +# summary, that old-timeline summary covers only WAL that never happened on +# the promoted server. An incremental backup taken after promotion must not +# use it: here it holds a VACUUM truncation that would discard the blocks +# modified on the old timeline before the switch point. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(allows_streaming => 1); +$primary->append_conf('postgresql.conf', <start; + +# One row per page, as in 013_summary_order.pl. The heap never exceeds six +# blocks, so it fits in one segment even with the RELSEG_SIZE used by CI. +my $block_size = $primary->safe_psql('postgres', 'SHOW block_size'); +my $pad_size = $block_size / 2; +$primary->safe_psql('postgres', <backup('full'); +my $full_path = $primary->backup_dir . '/full'; + +# Updating rows 3 and 4 modifies blocks 2 and 3 and extends the heap to six +# blocks. The standby replays this. The recovery target is the redo pointer +# of the following checkpoint, so it is also the start of a WAL summary. +my $size_sql = "SELECT pg_relation_size('t') / $block_size"; +$primary->safe_psql('postgres', "UPDATE t SET pad = pad || 'o' WHERE id > 2"); +is($primary->safe_psql('postgres', $size_sql), '6', + 'UPDATE extends the heap to six blocks'); +$primary->safe_psql('postgres', 'CHECKPOINT'); +my $target_lsn = $primary->safe_psql('postgres', + 'SELECT redo_lsn FROM pg_control_checkpoint()'); + +# The standby receives and summarizes this, but never replays it. +$primary->safe_psql('postgres', 'DELETE FROM t WHERE id > 2'); +$primary->safe_psql('postgres', 'VACUUM t'); +is($primary->safe_psql('postgres', $size_sql), '2', + 'VACUUM truncates the heap to two blocks after the recovery target'); +$primary->safe_psql('postgres', 'CHECKPOINT'); +my $end_lsn = $primary->safe_psql('postgres', + 'SELECT redo_lsn FROM pg_control_checkpoint()'); + +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, 'full', has_streaming => 1); +$standby->append_conf('postgresql.conf', <start; +$standby->poll_query_until('postgres', + "SELECT pg_get_wal_replay_pause_state() = 'paused'") + or die "timed out waiting for recovery to pause"; +$standby->poll_query_until('postgres', <= '$end_lsn') +EOF + or die "timed out waiting for a timeline 1 summary past $target_lsn"; +$primary->stop; +$standby->promote; + +# Don't read the table before the backup: with data checksums, setting hint +# bits would WAL-log those blocks again on the new timeline and hide the loss. +# Store both backups under $primary so init_from_backup() can combine them. +$standby->command_ok( + [ + 'pg_basebackup', '--no-sync', + '--pgdata' => $primary->backup_dir . '/incr', + '--checkpoint' => 'fast', + '--incremental' => "$full_path/backup_manifest", + ], + 'incremental backup after promotion'); + +my $contents_sql = "SELECT id, right(pad, 1) FROM t ORDER BY id"; +my $expected = "1|x\n2|x\n3|o\n4|o"; +is($standby->safe_psql('postgres', $contents_sql), + $expected, 'promoted standby has the rows updated before the target'); +$standby->stop; + +my $restored = PostgreSQL::Test::Cluster->new('restored'); +$restored->init_from_backup( + $primary, 'incr', + combine_with_prior => ['full'], + combine_mode => $mode); +$restored->start; +is($restored->safe_psql('postgres', $contents_sql), + $expected, 'restored rows match the promoted standby'); +$restored->stop; + +done_testing(); diff --git a/src/bin/pg_combinebackup/t/015_summary_past_switch.pl b/src/bin/pg_combinebackup/t/015_summary_past_switch.pl new file mode 100644 index 0000000000..b1c0bf8c17 --- /dev/null +++ b/src/bin/pg_combinebackup/t/015_summary_past_switch.pl @@ -0,0 +1,130 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group +# +# A streaming standby summarizes the WAL it receives, ahead of replay. When +# it is promoted at a recovery target inside a WAL summary, the old-timeline +# summary covering the target also covers WAL that the promoted server never +# replayed. Block references from that WAL are harmless, but a truncation +# recorded there must not be applied to an incremental backup. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(allows_streaming => 1); +$primary->append_conf('postgresql.conf', <start; + +# One row per page, so that the heap fits in one segment even with the +# six-block RELSEG_SIZE used by CI. +my $block_size = $primary->safe_psql('postgres', 'SHOW block_size'); +my $pad_size = $block_size / 2; +$primary->safe_psql('postgres', <backup('full'); +my $full_path = $primary->backup_dir . '/full'; + +# Each recovery target is inside a WAL summary: after the redo pointer of a +# checkpoint and some WAL that the standby replays. +my $insert_lsn = 'SELECT pg_current_wal_insert_lsn()'; +$primary->safe_psql('postgres', 'CHECKPOINT'); +$primary->safe_psql('postgres', "UPDATE t SET pad = pad || 'o' WHERE id > 2"); +my $target1 = $primary->safe_psql('postgres', $insert_lsn); + +# Past the first target, only new rows: harmless for that standby. +$primary->safe_psql('postgres', + "INSERT INTO t VALUES (5, repeat('x', $pad_size))"); +$primary->safe_psql('postgres', 'CHECKPOINT'); +$primary->safe_psql('postgres', 'CREATE TABLE marker ()'); +my $target2 = $primary->safe_psql('postgres', $insert_lsn); + +# Past the second target, a truncation from 7 blocks to 2. +$primary->safe_psql('postgres', 'DELETE FROM t WHERE id > 2'); +$primary->safe_psql('postgres', 'VACUUM t'); +is($primary->safe_psql('postgres', "SELECT pg_relation_size('t') / $block_size"), + '2', 'VACUUM truncates the heap after the second target'); +$primary->safe_psql('postgres', 'CHECKPOINT'); +my $end_lsn = $primary->safe_psql('postgres', + 'SELECT redo_lsn FROM pg_control_checkpoint()'); + +# Start a standby that pauses at the target and has summarized everything +# the primary wrote, then promote it. +sub promoted_standby +{ + my ($name, $target) = @_; + my $node = PostgreSQL::Test::Cluster->new($name); + $node->init_from_backup($primary, 'full', has_streaming => 1); + $node->append_conf('postgresql.conf', <start; + $node->poll_query_until('postgres', + "SELECT pg_get_wal_replay_pause_state() = 'paused'") + or die "timed out waiting for $name to pause"; + $node->poll_query_until('postgres', <= '$end_lsn') +EOF + or die "timed out waiting for $name to summarize up to $end_lsn"; + $node->promote; + return $node; +} + +my $standby1 = promoted_standby('standby1', $target1); +my $standby2 = promoted_standby('standby2', $target2); +$primary->stop; + +# The tables are not read before the backups: with data checksums, setting +# hint bits would WAL-log their blocks again on the new timeline. +my $contents_sql = "SELECT id, right(pad, 1) FROM t ORDER BY id"; + +# First standby: the summary past the switch only adds block references, so +# the incremental backup works and restores the promoted server's rows. +$standby1->command_ok( + [ + 'pg_basebackup', '--no-sync', + '--pgdata' => $primary->backup_dir . '/incr1', + '--checkpoint' => 'fast', + '--incremental' => "$full_path/backup_manifest", + ], + 'incremental backup when only new rows follow the target'); +my $expected1 = "1|x\n2|x\n3|o\n4|o"; +is($standby1->safe_psql('postgres', $contents_sql), + $expected1, 'first standby has the rows replayed before the target'); +$standby1->stop; +my $restored = PostgreSQL::Test::Cluster->new('restored'); +$restored->init_from_backup( + $primary, 'incr1', + combine_with_prior => ['full'], + combine_mode => $mode); +$restored->start; +is($restored->safe_psql('postgres', $contents_sql), + $expected1, 'restored rows match the first standby'); +$restored->stop; + +# Second standby: the summary past the switch records the truncation, which +# never happened on this server, so the incremental backup must refuse it. +$standby2->command_fails_like( + [ + 'pg_basebackup', '--no-sync', + '--pgdata' => $primary->backup_dir . '/incr2', + '--checkpoint' => 'fast', + '--incremental' => "$full_path/backup_manifest", + ], + qr/cannot use WAL summary file .* for an incremental backup/, + 'incremental backup refuses a truncation past the switch point'); +$standby2->stop; + +done_testing(); -- 2.56.0