Hi, KeepFileRestoredFromArchive() used to unlink an existing WAL segment before renaming the restored file into place on non-Windows. Between those two steps the segment is missing from pg_wal, so a concurrent walsender (or anything else looking at that path) can observe a gap.
On POSIX, durable_rename() already replaces the target atomically, so the prior unlink is unnecessary. Windows still needs the rename-to-.deletedN + unlink dance, so that path is unchanged. The attached patch drops the non-Windows unlink and adds a TAP test that uses an injection point between the old unlink site and the rename to show the segment remains present. Best regards, Stepan Neretin
From 126665d60b81583019ab0c1f7799d1c4936316d6 Mon Sep 17 00:00:00 2001 From: snppg <[email protected]> Date: Wed, 22 Jul 2026 19:26:03 +0700 Subject: [PATCH v1] Fix archive restore race that could unlink WAL before rename. --- src/backend/access/transam/xlogarchive.c | 11 +++-- src/test/recovery/meson.build | 1 + .../t/055_archive_restore_walsender_race.pl | 42 +++++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 src/test/recovery/t/055_archive_restore_walsender_race.pl diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 9a0c8097cb1..4e81eab85e5 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -31,6 +31,7 @@ #include "replication/walsender.h" #include "storage/fd.h" #include "storage/ipc.h" +#include "utils/injection_point.h" #include "utils/wait_event.h" /* @@ -366,9 +367,8 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (stat(xlogfpath, &statbuf) == 0) { - char oldpath[MAXPGPATH]; - #ifdef WIN32 + char oldpath[MAXPGPATH]; static unsigned int deletedcounter = 1; /* @@ -390,18 +390,17 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) errmsg("could not rename file \"%s\" to \"%s\": %m", xlogfpath, oldpath))); } -#else - /* same-size buffers, so this never truncates */ - strlcpy(oldpath, xlogfpath, MAXPGPATH); -#endif if (unlink(oldpath) != 0) ereport(FATAL, (errcode_for_file_access(), errmsg("could not remove file \"%s\": %m", xlogfpath))); +#endif reload = true; } + INJECTION_POINT("keepfile-restored-before-rename", NULL); + durable_rename(path, xlogfpath, ERROR); /* diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index ad0d85f4189..41a239f5a59 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -63,6 +63,7 @@ tests += { 't/052_checkpoint_segment_missing.pl', 't/053_standby_login_event_trigger.pl', 't/054_unlogged_sequence_promotion.pl', + 't/055_archive_restore_walsender_race.pl', ], }, } diff --git a/src/test/recovery/t/055_archive_restore_walsender_race.pl b/src/test/recovery/t/055_archive_restore_walsender_race.pl new file mode 100644 index 00000000000..c77fca449b5 --- /dev/null +++ b/src/test/recovery/t/055_archive_restore_walsender_race.pl @@ -0,0 +1,42 @@ +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +plan skip_all => 'injection points required' + unless $ENV{enable_injection_points} eq 'yes'; + +my $inj = 'keepfile-restored-before-rename'; +my $gate = "$PostgreSQL::Test::Utils::tmp_check/g$$"; + +my $p = PostgreSQL::Test::Cluster->new('p'); +$p->init( + has_archiving => 1, allows_streaming => 1, + extra => ['--wal-segsize', '1']); +$p->append_conf('postgresql.conf', + "shared_preload_libraries = 'injection_points'"); +$p->start; +$p->safe_psql('postgres', 'CREATE EXTENSION injection_points'); +$p->advance_wal(2); +$p->backup('b'); +$p->advance_wal(3); + +my $a = $p->archive_dir; +my $w = (sort grep { /^[0-9A-F]{24}$/ } slurp_dir($a))[-1]; +my $prev = substr($w, 0, 16) . sprintf('%08X', hex(substr($w, 16, 8)) - 1); +my $s = PostgreSQL::Test::Cluster->new('s'); +$s->init_from_backup($p, 'b', has_restoring => 1); +system('cp', "$a/$w", $s->data_dir . "/pg_wal/$w") == 0 or die $!; +$s->append_conf('postgresql.conf', qq{ +recovery_prefetch = off +shared_preload_libraries = 'injection_points' +restore_command = 'case "%f" in $w) while [ ! -f $gate ]; do sleep 0.01; done ;; esac; cp "$a/%f" "%p"' +}); +$s->start; +$s->wait_for_log(qr/restored log file "\Q$prev\E" from archive/); +$s->safe_psql('postgres', "SELECT injection_points_attach('$inj','wait')"); +system('touch', $gate); +$s->wait_for_event('startup', $inj); +ok(-e $s->data_dir . "/pg_wal/$w"); +done_testing(); -- 2.55.0
