From 1d3c2796674af1cef3bf34e4e7ccb6c96f74ae76 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 25 Oct 2026 10:46:39 -0400 Subject: [PATCH] Forget the prefetcher's current record in XLogPrefetcherBeginRead() XLogPrefetcherBeginRead() discards the queue of records etc. It however kept prefetcher->record, the record whose block references are examined by XLogPrefetcherNextBlock(). That pointer is cleared once all block references have been examined, or when XLogPrefetcherReadRecord() returns the record (see adb466150b4). A record that was decoded and not returned yet survived the reset. The next XLogPrefetcherReadRecord() call then continued with the remaining block references of that stale record, and decoded the record at the new read position only after those. That matters in FinishWalRecovery(), which re-reads the last replayed record after the redo loop to find out where WAL ends. When recovery stops at a recovery target, the prefetcher may be part-way through a later record. If the remaining block references of that record fill up the new I/O queue (4 * maintenance_io_concurrency entries, or maintenance_io_concurrency references that need I/O), no record is decoded at all, and: - Assert-enabled builds fail an assertion in XLogPrefetcherReadRecord() that expects the I/O queue to be empty when no records are queued, "lrq_completed(prefetcher->streaming_read) == 0" (or the lrq_inflight() one, if some of the block references needed I/O). - Other builds return NULL from ReadRecord() without an error, and FinishWalRecovery() treats the start of the last replayed record as the end of WAL. The history file of the new timeline is written with that wrong switch point, and the end-of-recovery checkpoint then fails with "xlog flush request ... is not satisfied" while writing a page modified by the last replayed record, followed by "FATAL: checkpoint request failed". The recovery signal file has already been removed at that point, so simply restarting the server performs crash recovery to the end of the WAL in pg_wal, past the recovery target. XLOG_FPI records written by bulk index builds, CLUSTER and similar operations carry up to XLR_MAX_BLOCK_ID block references that need no I/O. The prefetcher has examined at least one of them, so the rest can fill-up the I/O queue when maintenance_io_concurrency is set to 7 or less, if the recovery target is shortly before such a record. With higher settings, it needs a record with at least maintenance_io_concurrency remaining block references that need I/O, which is unusual. Fix by forgetting the current record in XLogPrefetcherBeginRead(). The other callers run before the redo loop, after reading a single record without any readahead (see begin_ptr), so the prefetcher has no current record at that point. Add a TAP test that stops recovery before the record preceding the first multi-block XLOG_FPI record of an SP-GiST index build, with maintenance_io_concurrency = 1, so that the prefetcher is part-way through the FPI record when recovery ends. The test uses SP-GiST rather than B-tree, and compares LSNs as pg_lsn rather than as strings, so that it works unchanged in the back branches: B-tree builds before 17 WAL-log each page in a separate record, and the server log in 15-18 doesn't zero-pad LSNs. Backpatch-through: 15 --- src/backend/access/transam/xlogprefetcher.c | 8 ++ src/test/recovery/meson.build | 1 + .../t/061_recovery_target_prefetch.pl | 94 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 src/test/recovery/t/061_recovery_target_prefetch.pl diff --git a/src/backend/access/transam/xlogprefetcher.c b/src/backend/access/transam/xlogprefetcher.c index dff57642ab4..5be2f34ef6f 100644 --- a/src/backend/access/transam/xlogprefetcher.c +++ b/src/backend/access/transam/xlogprefetcher.c @@ -975,6 +975,14 @@ XLogPrefetcherBeginRead(XLogPrefetcher *prefetcher, XLogRecPtr recPtr) prefetcher->no_readahead_until = InvalidXLogRecPtr; + /* + * Forget the record we were examining, if any. It is about to be + * discarded along with the rest of the decoder's queue, and continuing + * with its remaining block references could fill up the new I/O queue + * before the record at recPtr is decoded. + */ + prefetcher->record = NULL; + /* This will forget about any queued up records in the decoder. */ XLogBeginRead(prefetcher->reader, recPtr); } diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index b6ac9dd07a7..cb3df6ed895 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -69,6 +69,7 @@ tests += { 't/058_shutdown_crash_restart.pl', 't/059_remote_apply_status_interval.pl', 't/060_multixact_crashed_updater.pl', + 't/061_recovery_target_prefetch.pl', ], }, } diff --git a/src/test/recovery/t/061_recovery_target_prefetch.pl b/src/test/recovery/t/061_recovery_target_prefetch.pl new file mode 100644 index 00000000000..96f94d31b20 --- /dev/null +++ b/src/test/recovery/t/061_recovery_target_prefetch.pl @@ -0,0 +1,94 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that recovery ends correctly when a recovery target is reached while +# the WAL prefetcher is part-way through a later record with many block +# references. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node_primary = PostgreSQL::Test::Cluster->new('primary'); +$node_primary->init(has_archiving => 1, allows_streaming => 1); +$node_primary->start; + +$node_primary->safe_psql( + 'postgres', + "CREATE TABLE t (i int, v text); + INSERT INTO t SELECT i, md5(i::text) FROM generate_series(1, 10000) i;" +); +$node_primary->backup('backup'); + +# An SP-GiST build WAL-logs the new index with log_newpage_range(), in +# XLOG_FPI records with up to XLR_MAX_BLOCK_ID block references each. +my $start_lsn = + $node_primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()'); +$node_primary->safe_psql('postgres', + 'CREATE INDEX t_v ON t USING spgist (v)'); +my $end_lsn = + $node_primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()'); +my $walfile = $node_primary->safe_psql('postgres', + 'SELECT pg_walfile_name(pg_current_wal_lsn())'); +$node_primary->safe_psql('postgres', 'SELECT pg_switch_wal()'); +$node_primary->poll_query_until('postgres', + "SELECT last_archived_wal >= '$walfile' FROM pg_stat_archiver") + or die "timed out waiting for WAL to be archived"; + +# Stop recovery before the record preceding the first FPI record with many +# block references, rather than before the FPI record itself, because the +# prefetcher forgets about a record once it has been read (even if it is not +# replayed). With maintenance_io_concurrency = 1, the prefetcher only looks a +# few block references ahead, so it is part-way through the FPI record when +# recovery ends. +my ($stdout, $stderr) = run_command( + [ + 'pg_waldump', + '--path' => $node_primary->archive_dir, + '--start' => $start_lsn, + '--end' => $end_lsn, + ]); +$stdout =~ /prev ([0-9A-F]+\/[0-9A-F]+), desc: FPI , .*blkref #8: / + or die "could not find an FPI record with many block references"; +my $target_lsn = $1; + +my $node_standby = PostgreSQL::Test::Cluster->new('standby'); +$node_standby->init_from_backup( + $node_primary, 'backup', + has_restoring => 1, + standby => 0); +$node_standby->append_conf( + 'postgresql.conf', qq( +maintenance_io_concurrency = 1 +recovery_target_lsn = '$target_lsn' +recovery_target_inclusive = off +recovery_target_action = 'promote' +)); +$node_standby->start; +$node_standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()') + or die "timed out waiting for promotion"; + +# Compare the LSNs as pg_lsn, so as not to depend on how they're formatted. +my ($stop_lsn) = + slurp_file($node_standby->logfile) =~ + /recovery stopping before WAL location \(LSN\) "([0-9A-F]+\/[0-9A-F]+)"/ + or die "could not find where recovery stopped in the server log"; +is( $node_standby->safe_psql( + 'postgres', "SELECT '$stop_lsn'::pg_lsn = '$target_lsn'::pg_lsn"), + 't', + 'recovery stopped at the target'); +is( $node_standby->safe_psql( + 'postgres', "SELECT count(*), to_regclass('t_v') IS NULL FROM t"), + '10000|t', + 'data matches the recovery target'); + +# Check that the WAL written on the new timeline can be replayed. +$node_standby->safe_psql('postgres', 'INSERT INTO t VALUES (0)'); +$node_standby->stop('immediate'); +$node_standby->start; +is($node_standby->safe_psql('postgres', 'SELECT count(*) FROM t'), + '10001', 'crash recovery after promotion'); + +done_testing(); -- 2.55.0