I have incorporated all the feedback discussed above and attached the v3 patch. Please take a look and let me know if you have any additional feedback.
Best Regards, Nitin Jadhav Azure Database for PostgreSQL Microsoft
From ce9ef428d961a2de093be4a8d52b934be77cec60 Mon Sep 17 00:00:00 2001 From: Nitin Jadhav <[email protected]> Date: Wed, 10 Dec 2025 10:13:35 +0000 Subject: [PATCH] Fix crash during recovery when redo segment is missing The issue was that PostgreSQL did not handle the case when the redo LSN and checkpoint LSN were in separate segments, and the file containing the redo LSN was missing. This resulted in a crash. The fix ensures the REDO location exists after reading the checkpoint record in InitWalRecovery(). If the REDO location is missing, it logs a FATAL error. Additionally, a new test script, 050_redo_segment_missing.pl, has been added to validate this. This change also updates an existing error path: the previous PANIC emitted when ReadCheckpointRecord() fails in the no backup label case is now lowered to FATAL for consistency and improved operator experience. --- src/backend/access/transam/xlog.c | 2 + src/backend/access/transam/xlogrecovery.c | 12 +- src/test/recovery/meson.build | 1 + .../recovery/t/050_redo_segment_missing.pl | 106 ++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 src/test/recovery/t/050_redo_segment_missing.pl diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 22d0a2e8c3a..55d074c5389 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7153,6 +7153,8 @@ CreateCheckPoint(int flags) if (log_checkpoints) LogCheckpointStart(flags, false); + INJECTION_POINT("checkpoint-after-start", NULL); + /* Update the process title */ update_checkpoint_display(flags, false, false); diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 21b8f179ba0..8671e46355f 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -790,6 +790,16 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, ereport(DEBUG1, errmsg_internal("checkpoint record is at %X/%08X", LSN_FORMAT_ARGS(CheckPointLoc))); + + /* Make sure that REDO location exists. */ + if (RedoStartLSN < CheckPointLoc) + { + XLogPrefetcherBeginRead(xlogprefetcher, RedoStartLSN); + if (!ReadRecord(xlogprefetcher, LOG, false, RedoStartTLI)) + ereport(FATAL, + errmsg("could not find redo location %X/%08X referenced by checkpoint record at %X/%08X", + LSN_FORMAT_ARGS(RedoStartLSN), LSN_FORMAT_ARGS(CheckPointLoc))); + } } else { @@ -799,7 +809,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, * can't read the last checkpoint because this allows us to * simplify processing around checkpoints. */ - ereport(PANIC, + ereport(FATAL, errmsg("could not locate a valid checkpoint record at %X/%08X", LSN_FORMAT_ARGS(CheckPointLoc))); } diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index 523a5cd5b52..e93248bd66e 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -58,6 +58,7 @@ tests += { 't/047_checkpoint_physical_slot.pl', 't/048_vacuum_horizon_floor.pl', 't/049_wait_for_lsn.pl', + 't/050_redo_segment_missing.pl', ], }, } diff --git a/src/test/recovery/t/050_redo_segment_missing.pl b/src/test/recovery/t/050_redo_segment_missing.pl new file mode 100644 index 00000000000..92d217299d2 --- /dev/null +++ b/src/test/recovery/t/050_redo_segment_missing.pl @@ -0,0 +1,106 @@ +# Copyright (c) 2021-2025, PostgreSQL Global Development Group +# +# Evaluates PostgreSQL's recovery behavior when a WAL segment +# containing the redo record is missing. It initializes a +# PostgreSQL instance, configures it, and executes a series of +# operations to mimic a scenario where a WAL segment is absent. +# Then checks that PostgreSQL logs the correct error message +# when it fails to locate the redo record. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Time::HiRes qw(usleep); +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $psql_timeout = IPC::Run::timer($PostgreSQL::Test::Utils::timeout_default); +my $node = PostgreSQL::Test::Cluster->new('testnode'); +$node->init; +$node->start; + +# Check if the extension injection_points is available, as it may be +# possible that this script is run with installcheck, where the module +# would not be installed by default. +if (!$node->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} +$node->safe_psql('postgres', q(CREATE EXTENSION injection_points)); + +# Start a psql session to run the checkpoint in the background and make +# the test wait on the injection point so the checkpoint stops just after +# it starts. +my $checkpoint = $node->background_psql('postgres'); +$checkpoint->query( + q{select injection_points_attach('checkpoint-after-start','wait')} +); +$checkpoint->query_until( + qr/starting_checkpoint/, + q(\echo starting_checkpoint +checkpoint; +\q +)); + +# Wait until the checkpoint has reached the injection point. +note('waiting for injection_point'); +$node->wait_for_event('checkpointer', 'checkpoint-after-start'); +note('injection_point is reached'); + +# Switch the WAL to ensure that the WAL segment containing the checkpoint +# record is different from the one containing the redo record. +$node->safe_psql('postgres', 'SELECT pg_switch_wal()'); + +# Continue the checkpoint and wait for its completion. +my $log_offset = -s $node->logfile; +$node->safe_psql('postgres', + q{select injection_points_wakeup('checkpoint-after-start')}); +$node->wait_for_log(qr/checkpoint complete/, $log_offset); + +# Retrieve the WAL file names for the redo record and checkpoint record. +my $redo_lsn = $node->safe_psql('postgres', "SELECT redo_lsn FROM pg_control_checkpoint()"); +my $redo_walfile_name = $node->safe_psql('postgres', "SELECT pg_walfile_name('$redo_lsn')"); +my $checkpoint_lsn = $node->safe_psql('postgres', "SELECT checkpoint_lsn FROM pg_control_checkpoint()"); +my $checkpoint_walfile_name = $node->safe_psql('postgres', "SELECT pg_walfile_name('$checkpoint_lsn')"); + +# Die if both records are in the same WAL file. +if ($redo_walfile_name eq $checkpoint_walfile_name) { + die "redo record wal file is the same as checkpoint record wal file, aborting test"; +} + +# Remove the WAL segment containing the redo record +unlink $node->data_dir . "/pg_wal/$redo_walfile_name" + or die "Could not remove WAL file: $!"; + +$node->stop('immediate'); + +# Use run_log instead of node->start because this test expects +# that the server ends with an error during recovery. +run_log( + [ + 'pg_ctl', + '--pgdata' => $node->data_dir, + '--log' => $node->logfile, + 'start', + ]); + +# Wait for postgres to terminate +foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default) +{ + last if !-f $node->data_dir . '/postmaster.pid'; + usleep(100_000); +} + +# Confirm that the recovery fails with an expected error +my $logfile = slurp_file($node->logfile()); +ok( $logfile =~ + qr/FATAL: could not find redo location .* referenced by checkpoint record at .*/, + "ends with FATAL because it could not find redo location" +); + +done_testing(); \ No newline at end of file -- 2.43.0
