From 28bfea6760fcc044c76b2dc4df29278841f0d2f7 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Tue, 25 Aug 2026 15:06:58 +0000 Subject: [PATCH v4 4/4] pg_combinebackup: Refuse mixed data checksum states in a backup chain check_control_files() detected a chain whose backups were taken under different data checksum states, warned, and proceeded. The output directory keeps the last backup's control file, so a full backup taken with checksums off combined with an incremental taken after an offline enable produces a cluster whose control file says "on" while most of its blocks carry no checksums; it starts, and then every connection dies on the first unchecksummed catalog page. An offline enable between two backups of a chain is all it takes, since it rewrites every page without logging anything, so the incremental backup does not re-ship the pages. Turn the warning into an error, matching what pg_rewind does for the equivalent combinations. The check stays asymmetric on purpose: when the last backup was taken with checksums off, stale checksums from an earlier backup are never verified, and that chain remains usable. --- doc/src/sgml/ref/pg_combinebackup.sgml | 19 +-- src/bin/pg_combinebackup/pg_combinebackup.c | 14 +- src/test/modules/test_checksums/meson.build | 1 + .../t/028_combinebackup_mixed.pl | 134 ++++++++++++++++++ 4 files changed, 155 insertions(+), 13 deletions(-) create mode 100644 src/test/modules/test_checksums/t/028_combinebackup_mixed.pl diff --git a/doc/src/sgml/ref/pg_combinebackup.sgml b/doc/src/sgml/ref/pg_combinebackup.sgml index 9a6d201e0b8..f5d7e177b36 100644 --- a/doc/src/sgml/ref/pg_combinebackup.sgml +++ b/doc/src/sgml/ref/pg_combinebackup.sgml @@ -306,18 +306,19 @@ PostgreSQL documentation pg_combinebackup does not recompute page checksums when - writing the output directory. Therefore, if any of the backups used for - reconstruction were taken with checksums disabled, but the final backup was - taken with checksums enabled, the resulting directory may contain pages - with invalid checksums. + writing the output directory. It therefore refuses a chain in which some + of the backups used for reconstruction were taken with checksums disabled + but the final backup was taken with checksums enabled: the resulting + directory would contain pages with invalid checksums, and the cluster + would fail checksum verification as soon as it reads them. Take a new + full backup after enabling data checksums with + . - To avoid this problem, taking a new full backup after changing the checksum - state of the cluster using is - recommended. Otherwise, you can disable and then optionally reenable - checksums on the directory produced by pg_combinebackup - in order to correct the problem. + The reverse case is accepted: when the final backup was taken with + checksums disabled, stale checksums copied from an earlier backup are + never verified. diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 86e2ee37c40..254a27b125b 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -670,13 +670,19 @@ check_control_files(int n_backups, char **backup_dirs) pg_log_debug("system identifier is %" PRIu64, system_identifier); /* - * Warn the user if not all backups are in the same state with regards to - * checksums. + * Reject a chain whose backups were not all in the same state with + * regards to checksums. The loop above only flags that when the last + * backup has checksums enabled: the blocks taken from the older backups + * have no checksums, and pg_combinebackup does not recompute them, so the + * combined cluster would fail verification as soon as it reads them. The + * other direction is harmless, since stale checksums are never verified + * while checksums are disabled. */ if (data_checksum_mismatch) { - pg_log_warning("only some backups have checksums enabled"); - pg_log_warning_hint("Disable, and optionally reenable, checksums on the output directory to avoid failures."); + pg_log_error("only some backups have checksums enabled"); + pg_log_error_hint("Take a new full backup after changing the data checksum state with pg_checksums."); + exit(1); } return system_identifier; diff --git a/src/test/modules/test_checksums/meson.build b/src/test/modules/test_checksums/meson.build index 406cc946b0d..3db744859fc 100644 --- a/src/test/modules/test_checksums/meson.build +++ b/src/test/modules/test_checksums/meson.build @@ -51,6 +51,7 @@ tests += { 't/025_cascade_divergence.pl', 't/026_rewind_state.pl', 't/027_rewind_standby_target.pl', + 't/028_combinebackup_mixed.pl', ], }, } diff --git a/src/test/modules/test_checksums/t/028_combinebackup_mixed.pl b/src/test/modules/test_checksums/t/028_combinebackup_mixed.pl new file mode 100644 index 00000000000..ce9dee9413b --- /dev/null +++ b/src/test/modules/test_checksums/t/028_combinebackup_mixed.pl @@ -0,0 +1,134 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that pg_combinebackup refuses a chain whose backups were taken under +# different data checksum states when the final state has them enabled. +# +# The output directory keeps the last backup's control file, so a full +# backup taken with checksums off plus an incremental taken after an +# offline enable would produce a directory whose control file says "on" +# while most of its blocks have no checksums. An offline enable between +# the two backups is enough to get there: it rewrites every page but logs +# nothing, so the incremental backup does not re-ship the pages. +# +# The reverse order stays allowed: with the final backup taken with +# checksums off, stale checksums from an earlier backup are never verified. +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +use FindBin; +use lib $FindBin::RealBin; + +use DataChecksums::Utils; + +my $node = PostgreSQL::Test::Cluster->new('node'); +$node->init( + no_data_checksums => 1, + has_archiving => 1, + allows_streaming => 1); +$node->append_conf('postgresql.conf', 'summarize_wal = on'); +$node->append_conf('postgresql.conf', 'autovacuum = off'); +$node->start; + +$node->safe_psql('postgres', + "CREATE TABLE t1 AS SELECT generate_series(1,200000) AS a;"); +$node->safe_psql('postgres', 'CHECKPOINT;'); + +test_checksum_state($node, 'off'); + +$node->backup('full'); + +# Offline enable: rewrites every page, logs nothing. +$node->stop; +system_or_bail('pg_checksums', '--enable', '--pgdata', $node->data_dir); +$node->start; +test_checksum_state($node, 'on'); + +$node->safe_psql('postgres', + "CREATE TABLE t2 AS SELECT generate_series(1,1000) AS a;"); +$node->safe_psql('postgres', 'CHECKPOINT;'); + +$node->command_ok( + [ + 'pg_basebackup', + '--pgdata' => $node->backup_dir . '/incr', + '--dbname' => $node->connstr('postgres'), + '--no-sync', + '--checkpoint' => 'fast', + '--incremental' => $node->backup_dir . '/full/backup_manifest', + ], + 'incremental backup with checksums on'); + +# Combining the chain must be refused: the result would say "on" while the +# blocks inherited from the full backup have no checksums. +command_fails_like( + [ + 'pg_combinebackup', + $node->backup_dir . '/full', + $node->backup_dir . '/incr', + '--output' => $node->backup_dir . '/combined', + ], + qr/only some backups have checksums enabled/, + 'pg_combinebackup refuses a chain crossing an offline enable'); + +# The other direction: full backup with checksums on, offline disable, then +# an incremental. The last backup wins, so the combined cluster comes up off. +$node->backup('full2'); + +$node->stop; +system_or_bail('pg_checksums', '--disable', '--pgdata', $node->data_dir); +$node->start; +test_checksum_state($node, 'off'); + +$node->safe_psql('postgres', + "CREATE TABLE t3 AS SELECT generate_series(1,1000) AS a;"); +$node->safe_psql('postgres', 'CHECKPOINT;'); + +$node->command_ok( + [ + 'pg_basebackup', + '--pgdata' => $node->backup_dir . '/incr2', + '--dbname' => $node->connstr('postgres'), + '--no-sync', + '--checkpoint' => 'fast', + '--incremental' => $node->backup_dir . '/full2/backup_manifest', + ], + 'incremental backup with checksums off'); + +my $restored = PostgreSQL::Test::Cluster->new('restored'); +$restored->init_from_backup( + $node, 'incr2', + combine_with_prior => ['full2'], + has_restoring => 1, + standby => 0); + +$restored->start; + +my ($rc, $stdout, $stderr) = $restored->psql('postgres', + "SELECT setting FROM pg_settings WHERE name = 'data_checksums';"); +is($rc, 0, 'combined backup accepts connections') or diag("stderr: $stderr"); +is($stdout, 'off', 'combined backup follows the last backup state'); + +($rc, $stdout, $stderr) = + $restored->psql('postgres', 'SELECT count(*) FROM t3;'); +is($rc, 0, 'blocks from the incremental backup are readable') + or diag("stderr: $stderr"); + +($rc, $stdout, $stderr) = + $restored->psql('postgres', 'SELECT count(*) FROM t1;'); +is($rc, 0, 'blocks inherited from the full backup are readable') + or diag("stderr: $stderr"); + +my $log = PostgreSQL::Test::Utils::slurp_file($restored->logfile); +unlike( + $log, + qr/page verification failed/, + 'no checksum verification failures in the combined backup'); + +$restored->stop('immediate'); +$node->stop; + +done_testing(); -- 2.39.3 (Apple Git-146)