From df764ea49f4f5d6fe399a72e3f316e201f9526ac Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Wed, 21 Oct 2026 11:31:37 +0900 Subject: [PATCH v1] Fix incremental backup data loss due to WAL summary merge order An incremental backup taken after a relation is truncated and extended again can omit modified blocks. Combining it with an earlier backup can then restore stale data, causing live rows to disappear and deleted rows to reappear. WAL summaries were merged without ensuring LSN order. Applying a limit block from an older summary after processing a newer summary could discard references to blocks modified later, causing those blocks to be omitted from the backup. Sort the required summaries by ascending start LSN before merging them, preserving references to later block modifications so that the required blocks are included in the backup. Backpatch to 17, where incremental backup was introduced. Reported-by: Ke Author: Ke Author: Fujii Masao Discussion: https://www.postgresql.org/message-id/19774-8a1cbfd1774fcd0c%40postgresql.org Backpatch-through: 17 --- src/backend/backup/basebackup_incremental.c | 19 ++++ src/bin/pg_combinebackup/meson.build | 1 + .../pg_combinebackup/t/013_summary_order.pl | 94 +++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 src/bin/pg_combinebackup/t/013_summary_order.pl diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 364b3f87aba..c899d9a1328 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -142,6 +142,7 @@ static void manifest_process_wal_range(JsonManifestParseContext *context, pg_noreturn static void manifest_report_error(JsonManifestParseContext *context, const char *fmt, ...) pg_attribute_printf(2, 3); +static int compare_wal_summary_start_lsn(const ListCell *a, const ListCell *b); static int compare_block_numbers(const void *a, const void *b); /* @@ -560,6 +561,12 @@ PrepareForIncrementalBackup(IncrementalBackupInfo *ib, break; } + /* + * Merge summaries in ascending start LSN order so that applying a + * limit block does not discard block references from later summaries. + */ + list_sort(required_wslist, compare_wal_summary_start_lsn); + /* * Read all of the required block reference table files and merge all of * the data into a single in-memory block reference table. @@ -1045,6 +1052,18 @@ manifest_report_error(JsonManifestParseContext *context, const char *fmt, ...) errmsg_internal("%s", errbuf.data)); } +/* + * List comparator for WAL summary files, ordered by ascending start LSN. + */ +static int +compare_wal_summary_start_lsn(const ListCell *a, const ListCell *b) +{ + WalSummaryFile *ws1 = lfirst(a); + WalSummaryFile *ws2 = lfirst(b); + + return pg_cmp_u64(ws1->start_lsn, ws2->start_lsn); +} + /* * Quicksort comparator for block numbers. */ diff --git a/src/bin/pg_combinebackup/meson.build b/src/bin/pg_combinebackup/meson.build index ba1c8cfa3d0..10702f36a5d 100644 --- a/src/bin/pg_combinebackup/meson.build +++ b/src/bin/pg_combinebackup/meson.build @@ -40,6 +40,7 @@ tests += { 't/010_hardlink.pl', 't/011_ib_truncation.pl', 't/012_vm_consistency.pl', + 't/013_summary_order.pl', ], } } diff --git a/src/bin/pg_combinebackup/t/013_summary_order.pl b/src/bin/pg_combinebackup/t/013_summary_order.pl new file mode 100644 index 00000000000..426247f4d1a --- /dev/null +++ b/src/bin/pg_combinebackup/t/013_summary_order.pl @@ -0,0 +1,94 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group +# +# Check that an incremental backup preserves rows updated on a new +# timeline after VACUUM truncates the table on the previous timeline. +# Summaries are collected from newest to oldest timeline, but must be +# merged in ascending start LSN order to preserve references to blocks +# added after truncation. + +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; + +# Keep one row per page by storing a half-page payload without compression +# or out-of-line storage. Four pages fit 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', <safe_psql('postgres', 'VACUUM t'); + +my $size_sql = "SELECT pg_relation_size('t') / $block_size"; +is($primary->safe_psql('postgres', $size_sql), '4', + 'initial heap has four blocks'); + +# The backup's checkpoint excludes table creation from the WAL summaries +# needed later, avoiding a zero limit block that would force a full copy. +$primary->backup('full'); +my $full_path = $primary->backup_dir . '/full'; + +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, 'full', has_streaming => 1); +$standby->start; + +# Truncate the table on the old timeline before promoting the standby. +$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'); +$primary->wait_for_replay_catchup($standby); +$primary->stop; +$standby->promote; + +# Updating the remaining rows requires new pages, re-extending the heap. +$standby->safe_psql('postgres', "UPDATE t SET pad = pad || 'u'"); +is($standby->safe_psql('postgres', $size_sql), '4', + 'UPDATE re-extends the heap to four blocks'); + +# Check IDs and values: the deleted rows could reappear without changing +# the row count. +my $contents_sql = + "SELECT id, pad = (repeat('x', $pad_size) || 'u') FROM t ORDER BY id"; +my $expected = "1|t\n2|t"; +is($standby->safe_psql('postgres', $contents_sql), + $expected, 'source contains the two updated rows'); + +# 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 truncation and re-extension'); +$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 updated source rows'); +$restored->stop; + +done_testing(); -- 2.56.0