From de15c3cb03893171f308b4f180103c89e59a6432 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Mon, 17 Aug 2026 10:56:20 +0200 Subject: [PATCH v4 2/2] Review hackery --- src/backend/access/transam/xlog.c | 6 +- .../test_checksums/t/010_backup_straddle.pl | 61 ++++++++++++++----- .../test_checksums/t/011_standby_straddle.pl | 51 +++++++++++----- 3 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 7d18bc3a973..4cbc91cad96 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -9297,9 +9297,9 @@ xlog2_redo(XLogReaderState *record) * recover back up to this point before allowing hot standby again. * The new state is durable in pg_control while its location is only * tracked in shared memory; a standby becoming consistent below this - * record would let base backups resume checksum verification with - * the location unknown. The local copies cannot be updated as long - * as crash recovery is happening and we expect all the WAL to be + * record would let base backups resume checksum verification with the + * location unknown. The local copies cannot be updated as long as + * crash recovery is happening and we expect all the WAL to be * replayed. */ if (InArchiveRecovery) diff --git a/src/test/modules/test_checksums/t/010_backup_straddle.pl b/src/test/modules/test_checksums/t/010_backup_straddle.pl index db50cd81fcd..fd205fd3bb8 100644 --- a/src/test/modules/test_checksums/t/010_backup_straddle.pl +++ b/src/test/modules/test_checksums/t/010_backup_straddle.pl @@ -22,6 +22,7 @@ use warnings FATAL => 'all'; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; +use File::Path qw(rmtree); use Test::More; use IPC::Run; @@ -68,12 +69,15 @@ my $backupdir = $node->backup_dir . '/straddle'; my ($out, $err) = ('', ''); my $backup = IPC::Run::start( [ - 'pg_basebackup', '-D', $backupdir, - '--wal-method=none', '--no-sync', - '--checkpoint=fast', + 'pg_basebackup', '-D', + $backupdir, '--wal-method=none', + '--no-sync', '--checkpoint=fast', '-d', $node->connstr('postgres') ], - '>', \$out, '2>', \$err, + '>', + \$out, + '2>', + \$err, IPC::Run::timeout(180)); $node->wait_for_event('walsender', 'basebackup-before-send-files'); @@ -99,7 +103,7 @@ $node->safe_psql('postgres', wait_for_checksum_state($node, 'on'); $node->poll_query_until('postgres', - "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " + "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " . "WHERE backend_type = 'datachecksums launcher';"); my $result = $node->safe_psql('postgres', @@ -110,10 +114,12 @@ is($result, '0', 'no spurious checksum failures after enable'); # A backup started once enabling has completed must verify, and pass $node->command_ok( [ - 'pg_basebackup', '-D', $node->backup_dir . '/after_enable', - '--wal-method=none', '--no-sync', '--checkpoint=fast' + 'pg_basebackup', '-D', + $node->backup_dir . '/after_enable', '--wal-method=none', + '--no-sync', '--checkpoint=fast' ], 'backup after enable completion succeeds'); +rmtree($node->backup_dir . '/after_enable'); # Now test a backup which straddles checksums being disabled and re-enabled. # Recreate the table since the earlier scan set its hint bits and the rewrite @@ -130,12 +136,15 @@ $backupdir = $node->backup_dir . '/onoffon'; ($out, $err) = ('', ''); $backup = IPC::Run::start( [ - 'pg_basebackup', '-D', $backupdir, - '--wal-method=none', '--no-sync', - '--checkpoint=fast', + 'pg_basebackup', '-D', + $backupdir, '--wal-method=none', + '--no-sync', '--checkpoint=fast', '-d', $node->connstr('postgres') ], - '>', \$out, '2>', \$err, + '>', + \$out, + '2>', + \$err, IPC::Run::timeout(180)); $node->wait_for_event('walsender', 'basebackup-before-send-files'); @@ -172,21 +181,45 @@ $node->safe_psql('postgres', wait_for_checksum_state($node, 'on'); $node->poll_query_until('postgres', - "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " + "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " . "WHERE backend_type = 'datachecksums launcher';"); $result = $node->safe_psql('postgres', "SELECT coalesce(sum(checksum_failures), 0) FROM pg_catalog.pg_stat_database;" ); is($result, '0', 'no spurious checksum failures after disable and re-enable'); +rmtree($backupdir); # A backup started once re-enabling has completed must verify, and pass $node->command_ok( [ - 'pg_basebackup', '-D', $node->backup_dir . '/after_onoffon', - '--wal-method=none', '--no-sync', '--checkpoint=fast' + 'pg_basebackup', '-D', + $node->backup_dir . '/after_onoffon', '--wal-method=none', + '--no-sync', '--checkpoint=fast' ], 'backup after re-enable completion succeeds'); +rmtree($node->backup_dir . '/after_onoffon'); + +# Corrupt data on disk and take another backup to make sure the corruption is +# reported +my $fcorrupt = $node->safe_psql('postgres', + q{CREATE TABLE corrupt AS SELECT a FROM generate_series(1,10000) AS a; ALTER TABLE corrupt SET (autovacuum_enabled=false); SELECT pg_relation_filepath('corrupt')} +); +$node->stop; +$node->corrupt_page_checksum($fcorrupt, 0); +$node->start; + +$node->command_checks_all( + [ + 'pg_basebackup', '-D', + $node->backup_dir . '/after_onoffon_corrupt', '--wal-method=none', + '--no-sync', '--checkpoint=fast' + ], + 1, + [qr{^$}], + [qr/^WARNING.*checksum verification failed/s], + 'pg_basebackup reports checksum mismatch'); +rmtree($node->backup_dir . '/after_onoffon_corrupt'); $node->stop; done_testing(); diff --git a/src/test/modules/test_checksums/t/011_standby_straddle.pl b/src/test/modules/test_checksums/t/011_standby_straddle.pl index fd6d635c519..2a232995af3 100644 --- a/src/test/modules/test_checksums/t/011_standby_straddle.pl +++ b/src/test/modules/test_checksums/t/011_standby_straddle.pl @@ -28,6 +28,7 @@ use warnings FATAL => 'all'; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; +use File::Path qw(rmtree); use Test::More; use FindBin; @@ -95,19 +96,36 @@ $node_standby->safe_psql('postgres', 'CHECKPOINT;'); # standby's copies of them can be removed later, and remember where the # enabling era begins. $node_primary->safe_psql('postgres', 'SELECT pg_switch_wal();'); -my $enable_start_lsn = - $node_primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn();'); -my $enable_start_seg = $node_primary->safe_psql('postgres', - "SELECT pg_walfile_name('$enable_start_lsn');"); # Enable checksums, holding the launcher after the state change but before # the final checkpoint, so the rewritten pages stay dirty everywhere. +$node_primary->safe_psql('postgres', + "SELECT injection_points_attach('datachecksums-enable-checksums-delay', 'wait');" +); $node_primary->safe_psql('postgres', "SELECT injection_points_attach('datachecksums-on-before-checkpoint','wait');" ); enable_data_checksums($node_primary); + +$node_primary->wait_for_event('datachecksums launcher', + 'datachecksums-enable-checksums-delay'); +my $enable_start_lsn = + $node_primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn();'); +my $enable_start_seg = $node_primary->safe_psql('postgres', + "SELECT pg_walfile_name('$enable_start_lsn');"); +$node_primary->safe_psql('postgres', + "SELECT injection_points_wakeup('datachecksums-enable-checksums-delay');" +); + +# Immediately start to wait for the next event and hold off on detaching the +# previous injection point till later to avoid delays and risk missing the +# wait event $node_primary->wait_for_event('datachecksums launcher', 'datachecksums-on-before-checkpoint'); +# Detach the injection point now that we have some more time +$node_primary->safe_psql('postgres', + "SELECT injection_points_detach('datachecksums-enable-checksums-delay');" +); # The standby has now replayed the state change: its pg_control says "on" # while the rewritten pages are only dirty in its shared buffers. @@ -126,11 +144,10 @@ my ($stdout, $stderr) = my ($min_recovery) = $stdout =~ /Minimum recovery ending location:\s*([0-9A-F]+\/[0-9A-F]+)/; die "could not parse pg_controldata output" unless defined $min_recovery; -is( $node_primary->safe_psql( - 'postgres', - "SELECT '$min_recovery'::pg_lsn > '$enable_start_lsn'::pg_lsn;"), - 't', - 'minRecoveryPoint advanced past the checksum state change'); + +my $result = $node_primary->safe_psql('postgres', + "SELECT '$min_recovery'::pg_lsn > '$enable_start_lsn'::pg_lsn;"); +is($result, 't', 'minRecoveryPoint advanced past the checksum state change'); # Remove the enabling-era WAL from the standby and cut it off from the # primary, so that replay after the restart stalls below the state change. @@ -173,15 +190,17 @@ $node_primary->wait_for_catchup($node_standby, 'replay', # pages still lack checksums. A base backup must skip verification and pass. $node_standby->command_ok( [ - 'pg_basebackup', '-D', $node_standby->backup_dir . '/underway', - '--wal-method=none', '--no-sync', '--checkpoint=fast' + 'pg_basebackup', '-D', + $node_standby->backup_dir . '/underway', '--wal-method=none', + '--no-sync', '--checkpoint=fast' ], 'backup from standby while enabling is underway succeeds'); -my $result = $node_standby->safe_psql('postgres', +$result = $node_standby->safe_psql('postgres', "SELECT coalesce(sum(checksum_failures), 0) FROM pg_catalog.pg_stat_database;" ); is($result, '0', 'no spurious checksum failures while enabling is underway'); +rmtree($node_standby->backup_dir . '/underway'); # Release the enabling; its final checkpoint flushes the rewritten pages. $node_primary->safe_psql('postgres', @@ -190,7 +209,7 @@ $node_primary->safe_psql('postgres', "SELECT injection_points_detach('datachecksums-on-before-checkpoint');"); wait_for_checksum_state($node_primary, 'on'); $node_primary->poll_query_until('postgres', - "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " + "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " . "WHERE backend_type = 'datachecksums launcher';"); # A restartpoint on the final checkpoint lets verification resume, and a @@ -201,10 +220,12 @@ $node_standby->safe_psql('postgres', 'CHECKPOINT;'); $node_standby->command_ok( [ - 'pg_basebackup', '-D', $node_standby->backup_dir . '/after_enable', - '--wal-method=none', '--no-sync', '--checkpoint=fast' + 'pg_basebackup', '-D', + $node_standby->backup_dir . '/after_enable', '--wal-method=none', + '--no-sync', '--checkpoint=fast' ], 'backup from standby after enable completion succeeds'); +rmtree($node_standby->backup_dir . '/after_enable'); $result = $node_standby->safe_psql('postgres', "SELECT coalesce(sum(checksum_failures), 0) FROM pg_catalog.pg_stat_database;" -- 2.39.3 (Apple Git-146)