From 0cb425fd69c9ab07e8084588accd2da164e45451 Mon Sep 17 00:00:00 2001 From: Daria Lepikhova Date: Mon, 28 Sep 2026 16:48:06 +0200 Subject: [PATCH v3] 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. Also free the path that GetFileBackupMethod() builds to look up an incremental file, which was never freed and is now built twice per file. Add a TAP test comparing the totals reported for a full and an incremental backup. --- doc/src/sgml/protocol.sgml | 14 ++- doc/src/sgml/ref/pg_basebackup.sgml | 4 +- src/backend/backup/basebackup.c | 11 ++- src/backend/backup/basebackup_incremental.c | 5 +- src/bin/pg_basebackup/meson.build | 1 + .../t/012_incremental_progress.pl | 96 +++++++++++++++++++ src/include/backup/basebackup_sink.h | 7 +- 7 files changed, 124 insertions(+), 14 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..2ee01f63990 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. @@ -3425,8 +3427,14 @@ 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 size of the archive that will be generated for the + tablespace, in kilobytes (1024 bytes), before any server-side + compression, 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 a relation file that has not changed + since the prior backup, or has changed only in part, is replaced by + a smaller incremental file containing just the blocks that have + changed. diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 3117968d125..b4a0faccf41 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -725,8 +725,8 @@ PostgreSQL documentation always being NULL. - Without this option, the backup will start by enumerating - the size of the entire database, and then go back and send + Without this option, the backup will start by estimating the total + amount of data that will be streamed, and then go back and send the actual contents. This may make the backup take slightly longer, and in particular it will take longer before the first data is sent. This option is useful to avoid such estimation 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/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 27fc42a2a43..ac4944c06a0 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -723,10 +723,13 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path, if (backup_file_lookup(ib->manifest_files, path) == NULL) { char *ipath; + bool found; ipath = GetIncrementalFilePath(dboid, spcoid, relfilenumber, forknum, segno); - if (backup_file_lookup(ib->manifest_files, ipath) == NULL) + found = backup_file_lookup(ib->manifest_files, ipath) != NULL; + pfree(ipath); + if (!found) return BACK_UP_FILE_FULLY; } 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..d63066d0840 --- /dev/null +++ b/src/bin/pg_basebackup/t/012_incremental_progress.pl @@ -0,0 +1,96 @@ +# 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; + +# Most of the data that will not change goes into a tablespace of its own, so +# that the estimate is exercised both for $PGDATA and for a tablespace. An +# in-place tablespace needs no mapping when the backup is taken in tar format. +$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 failed: $stderr"; + + # Progress lines are separated by carriage returns and look like + # " 6519/346720 kB (1%), 1/2 tablespaces". + my @lines = grep { /kB/ } split(/[\r\n]+/, $stderr); + ok(@lines > 0, "progress was reported for $path") + or die "no progress output for $path"; + + my ($streamed, $total) = $lines[-1] =~ m{(\d+)/(\d+) kB}; + defined($total) + or die "could not parse progress line for $path: $lines[-1]"; + return ($streamed, $total); +} + +my $backup_dir = $node->backup_dir; +my ($full_streamed, $full_total) = reported_progress("$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 streamed $full_streamed of $full_total kB"; +note "incremental backup streamed $incr_streamed 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' +); + +# The estimate should also match what is really sent, in $PGDATA and in the +# tablespace alike. pg_basebackup raises the total to the amount streamed, so +# the two end up equal when the estimate is right, and the total stays far +# above it when any part of the cluster is still counted in full. +cmp_ok($incr_total, '<', $incr_streamed * 2, + 'incremental backup estimate is close to the amount streamed'); + +done_testing(); diff --git a/src/include/backup/basebackup_sink.h b/src/include/backup/basebackup_sink.h index 96fa2c4eaba..b282cce3fd4 100644 --- a/src/include/backup/basebackup_sink.h +++ b/src/include/backup/basebackup_sink.h @@ -51,10 +51,11 @@ typedef struct bbsink_ops bbsink_ops; * 'tablespace_num' is the index of the current tablespace within the list * stored in 'tablespaces'. * - * 'bytes_done' is the number of bytes read so far from $PGDATA. + * 'bytes_done' is the number of bytes of archive data generated so far. This + * is counted before any server-side compression. * - * 'bytes_total' is the total number of bytes estimated to be present in - * $PGDATA, if we have estimated this. + * 'bytes_total' is the total number of bytes of archive data estimated to be + * generated, if we have estimated this. * * 'bytes_total_is_valid' is true if and only if a proper estimate has been * stored into 'bytes_total'. base-commit: 8e82d6610e7280d7870c0c36ff1c797e3cfb3e09 -- 2.50.1 (Apple Git-155)