From 985644d05e01af9eecd9f3a967f4972ab26b5da1 Mon Sep 17 00:00:00 2001 From: Daria Lepikhova Date: Mon, 28 Sep 2026 16:48:06 +0200 Subject: [PATCH v4 2/2] Report correct size estimate for incremental backups Progress reporting in pg_basebackup and pg_stat_progress_basebackup used the estimated size of a full backup even when taking an incremental backup. The size estimation pass did not account for incremental files and counted every file at its full size. Progress could therefore remain at only a few percent even after the incremental backup completed successfully. Fix this by making the size estimation pass account for incremental files, counting them at the size they will occupy in the archive. Backpatch to 17, where incremental backup was introduced. Discussion: https://postgr.es/m/CAK49S1ASwk7L-7LYUUz1MU_+uQnZ75srva2XRR4r-w0HCVZeUQ@mail.gmail.com Backpatch-through: 17 --- doc/src/sgml/protocol.sgml | 9 +- src/backend/backup/basebackup.c | 7 +- src/backend/backup/basebackup_incremental.c | 4 + src/bin/pg_basebackup/meson.build | 1 + .../t/012_incremental_progress.pl | 85 +++++++++++++++++++ 5 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 src/bin/pg_basebackup/t/012_incremental_progress.pl diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index e3839518b13..84af9372ebe 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -3200,7 +3200,9 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" before the transfer is even started, and might as such have a negative impact on the performance. In particular, it might take longer before the first data - is streamed. Since the database files can change during the backup, + is streamed. For an incremental backup, each file is counted at the + size it will occupy in the archive rather than its size on disk. + Since the database files can change during the backup, the size is only approximate and might both grow and shrink between the time of approximation and the sending of the actual files. The default is false. @@ -3429,6 +3431,11 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" tablespace, in kilobytes (1024 bytes), if progress report has been requested; otherwise it's null. This size is measured before any server-side compression. + For an incremental backup this can be much smaller than the + size of the tablespace, since a relation file that has not changed + since the prior backup, or has changed only in part, can be replaced + by a smaller incremental file containing just the blocks that have + changed. diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index e3c04ecd810..d7c485dc002 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -315,10 +315,10 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, if (tmp->path == NULL) tmp->size = sendDir(sink, ".", 1, true, state.tablespaces, - true, NULL, InvalidOid, NULL); + true, NULL, InvalidOid, ib); else tmp->size = sendTablespace(sink, tmp->path, tmp->oid, true, - NULL, NULL); + NULL, ib); state.bytes_total += tmp->size; } state.bytes_total_is_valid = true; @@ -1183,7 +1183,8 @@ sendTablespace(bbsink *sink, char *path, Oid spcoid, bool sizeonly, /* * Include all files from the given directory in the output tar stream. If * 'sizeonly' is true, we just calculate a total length and return it, without - * actually sending anything. + * actually sending anything. A file that will be sent incrementally is + * counted at the size of the incremental file. * * Omit any directory in the tablespaces list, to avoid backing up * tablespaces twice when they were created inside PGDATA. diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 27fc42a2a43..364b3f87aba 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -727,7 +727,11 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path, ipath = GetIncrementalFilePath(dboid, spcoid, relfilenumber, forknum, segno); if (backup_file_lookup(ib->manifest_files, ipath) == NULL) + { + pfree(ipath); return BACK_UP_FILE_FULLY; + } + pfree(ipath); } /* diff --git a/src/bin/pg_basebackup/meson.build b/src/bin/pg_basebackup/meson.build index d70ce5786a2..2d3b22ff06e 100644 --- a/src/bin/pg_basebackup/meson.build +++ b/src/bin/pg_basebackup/meson.build @@ -100,6 +100,7 @@ tests += { 'tests': [ 't/010_pg_basebackup.pl', 't/011_in_place_tablespace.pl', + 't/012_incremental_progress.pl', 't/020_pg_receivewal.pl', 't/030_pg_recvlogical.pl', 't/040_pg_createsubscriber.pl', diff --git a/src/bin/pg_basebackup/t/012_incremental_progress.pl b/src/bin/pg_basebackup/t/012_incremental_progress.pl new file mode 100644 index 00000000000..fdc0729cf58 --- /dev/null +++ b/src/bin/pg_basebackup/t/012_incremental_progress.pl @@ -0,0 +1,85 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Verify that incremental backup size estimates account for files sent in +# incremental format, rather than counting every file at its full size. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init(allows_streaming => 1); +$node->append_conf('postgresql.conf', 'summarize_wal = on'); +$node->append_conf('postgresql.conf', 'autovacuum = off'); +$node->start; + +# Put unchanged data in a separate tablespace to exercise size estimation +# for both PGDATA and user-defined tablespaces. +$node->safe_psql('postgres', < $node->host, + '--port' => $node->port, + '--pgdata' => $path, + '--format' => 'tar', + '--wal-method' => 'none', + '--no-sync', + '--progress', + '--checkpoint' => 'fast', + @extra + ], + '>' => \$stdout, + '2>' => \$stderr); + ok($result, "backup into $path succeeded") + or die "pg_basebackup into $path failed: $stderr"; + + # Use the last progress line, allowing either newline or carriage return. + my @lines = grep { /kB/ } split(/[\r\n]+/, $stderr); + my $line = $lines[-1] // ''; + my ($streamed, $total) = $line =~ m{^\s*(\d+)/(\d+) kB\b}; + ok(defined($total), "progress was reported for $path") + or die "could not parse progress output for $path: $stderr"; + note "backup into $path streamed $streamed of $total kB"; + return ($streamed, $total); +} + +my $backup_dir = $node->backup_dir; +my (undef, $full_total) = backup_with_progress("$backup_dir/full"); + +# Make a small change before taking the incremental backup. +$node->safe_psql('postgres', + 'INSERT INTO changed SELECT g FROM generate_series(1, 1000) g'); + +my ($incr_streamed, $incr_total) = backup_with_progress("$backup_dir/incr", + '--incremental' => "$backup_dir/full/backup_manifest"); + +# The incremental backup estimate should be much smaller than the full one. +cmp_ok($incr_total, '<', $full_total / 2, + 'incremental backup estimate is much smaller than a full backup estimate' +); + +# Check for a substantial overestimate. pg_basebackup raises the displayed +# total when the amount streamed exceeds the original estimate. +cmp_ok( + $incr_total, '<', + $incr_streamed * 2, + 'incremental backup estimate is not substantially overestimated'); + +done_testing(); -- 2.56.0