From 960cec213a1b269c6fddf451a221b3f1f3b4db15 Mon Sep 17 00:00:00 2001 From: Daria Lepikhova Date: Mon, 28 Sep 2026 16:48:06 +0200 Subject: [PATCH v1] Report correct size estimate for incremental backups The estimate behind pg_basebackup --progress and pg_stat_progress_basebackup was computed by a size-only pass over the directory tree that was never told the backup is incremental, so every file was counted at its full size. A finished incremental backup could therefore sit at a few percent, even though backup_type already reported it as incremental. Pass the IncrementalBackupInfo to that pass, so that files which will be sent incrementally are counted at the size they will actually take. The size reported for a tablespace is now the amount of data expected to be sent rather than the size of the directory, so update the protocol documentation. Add a TAP test comparing the totals reported for a full and an incremental backup. --- doc/src/sgml/protocol.sgml | 11 ++- src/backend/backup/basebackup.c | 11 +-- src/bin/pg_basebackup/meson.build | 1 + .../t/012_incremental_progress.pl | 80 +++++++++++++++++++ 4 files changed, 95 insertions(+), 8 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 8ced69f283b..66a40b2e18c 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 backup 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. @@ -3425,8 +3427,11 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" size (int8) - The approximate size of the tablespace, in kilobytes (1024 bytes), - if progress report has been requested; otherwise it's null. + The approximate amount of data that will be sent for the + tablespace, in kilobytes (1024 bytes), if progress report has been + requested; otherwise it's null. For an incremental backup this can + be much smaller than the size of the tablespace, since files that + have changed only partially are sent as incremental files. diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index e3c04ecd810..d674fb27543 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -302,8 +302,8 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, state.tablespaces = lappend(state.tablespaces, newti); /* - * Calculate the total backup size by summing up the size of each - * tablespace + * Calculate the total backup size by summing up the amount of data + * that will be sent for each tablespace */ if (opt->progress) { @@ -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/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..019abc0fcc1 --- /dev/null +++ b/src/bin/pg_basebackup/t/012_incremental_progress.pl @@ -0,0 +1,80 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Verify that the backup size estimate reported for an incremental backup +# reflects the amount of data that will actually be sent, rather than the size +# of a full backup. + +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; + +# Enough data that a full backup and an incremental one differ clearly in +# size, without writing more than necessary. +$node->safe_psql('postgres', < $node->host, + '--port' => $node->port, + '--pgdata' => $path, + '--no-sync', + '--progress', + '--checkpoint' => 'fast', + @extra + ], + '>' => \$stdout, + '2>' => \$stderr); + ok($result, "backup into $path succeeded") or diag $stderr; + + # Progress lines are separated by carriage returns and look like + # " 6519/346720 kB (1%), 1/1 tablespace". + my @lines = grep { /kB/ } split(/[\r\n]+/, $stderr); + ok(@lines > 0, "progress was reported for $path") or return undef; + + my ($total) = $lines[-1] =~ m{\d+/(\d+) kB}; + return $total; +} + +my $backup_dir = $node->backup_dir; +my $full_total = reported_total("$backup_dir/full"); + +# A small change, so that the incremental backup has very little to send. +$node->safe_psql('postgres', < "$backup_dir/full/backup_manifest"); + +note "full backup reported a total of $full_total kB"; +note "incremental backup reported a total of $incr_total kB"; + +# An incremental backup sends only a fraction of the cluster, so an estimate +# that still describes a full backup is far too large. +cmp_ok($incr_total, '<', $full_total / 2, + 'incremental backup estimate is much smaller than a full backup estimate' +); + +done_testing(); -- 2.50.1 (Apple Git-155)