From d3ed599a3a80ff253133ebbf104652b5fab3cb06 Mon Sep 17 00:00:00 2001 From: Manu Date: Fri, 23 Oct 2026 22:15:53 -0300 Subject: [PATCH 2/3] 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. Commit f549ecd2660 stopped using an old-timeline summary that starts exactly at the switch point, and commit 316861101e8 refuses a prior backup whose WAL range extends past it. This covers a summary that starts before the switch point and ends after it. Tested-by: Sai Pranav Discussion: https://postgr.es/m/CAHGQGwFbpzR2Wkrku9J8w6E+-rzD6Ovazs8n_kxvzfccfnJ0tg@mail.gmail.com --- src/backend/backup/basebackup_incremental.c | 43 +++++- src/bin/pg_combinebackup/meson.build | 1 + .../t/015_summary_past_switch.pl | 130 ++++++++++++++++++ 3 files changed, 173 insertions(+), 1 deletion(-) 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 233da98043c..50b5fcf5ff2 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -267,7 +267,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; @@ -566,6 +567,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. @@ -591,6 +615,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; @@ -609,6 +634,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 64da66b338e..b3d28ca866c 100644 --- a/src/bin/pg_combinebackup/meson.build +++ b/src/bin/pg_combinebackup/meson.build @@ -42,6 +42,7 @@ tests += { 't/012_vm_consistency.pl', 't/013_summary_order.pl', 't/014_vm_truncation.pl', + 't/015_summary_past_switch.pl', ], } } 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 00000000000..b1c0bf8c171 --- /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.37.1 (Apple Git-137.1)