From 1200dfd60c367e97a5e1f31100d82072eb0178b6 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 6 Aug 2026 08:19:13 +0900 Subject: [PATCH] Remove redundant unlink() in KeepFileRestoredFromArchive() for non-WIN32 KeepFileRestoredFromArchive() called unlink() on an existing WAL segment before durable_rename() replaced it. rename(2) atomically replaces a target on POSIX systems, and the extra unlink() created a small window where it would be possible for a concurrent WAL sender to see a segment as missing. Note that the window still exists on WIN32, as we lack a safe concurrent alternative. Perhaps something like ReplaceFile() could be looked at for this purpose. This problem is hard to reach in practice, so no backpatch is done. I also tend to be conservative regarding recovery changes, even small, in stable branches. Author: Stepan Neretin Reviewed-by: Andrey Borodin Reviewed-by: Neil Chen Discussion: https://postgr.es/m/CA+Yyo5S0C25jS_pRWQiGgoiy+X=GyJS7ZJkEq=_SW1WWpkL3Sg@mail.gmail.com --- src/backend/access/transam/xlogarchive.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 9a0c8097cb1..16940927af0 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -366,9 +366,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; /* @@ -380,6 +379,10 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * name first. Use a counter to create a unique filename, because the * same file might be restored from the archive multiple times, and a * walsender could still be holding onto an old deleted version of it. + * + * Note that this leaves a short window where the target name does not + * exist, during which a concurrent walsender opening the segment + * could fail. */ snprintf(oldpath, MAXPGPATH, "%s.deleted%u", xlogfpath, deletedcounter++); @@ -390,15 +393,13 @@ 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; }