0001-Fix-Windows-basebackup-by-renaming-before-unlinking.patch

From 5990d14bb4f9a0ca83fd1b6c7c6e0a6a23786a0d Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sun, 5 Sep 2021 17:07:44 +1200
Subject: [PATCH] Fix Windows basebackup by renaming before unlinking.

Rename files before unlinking on Windows.  This solves a problem where
basebackup could fail because other backends have a file handle to a
deleted file, so stat() and open() fail with ERROR_ACCESS_DENIED.

Suggested by Kyotaro Horiguchi <horikyota.ntt@gmail.com>.

XXX This is only a rough sketch to try the idea out!

Discussion: https://postgr.es/m/CA%2BhUKGJz_pZTF9mckn6XgSv69%2BjGwdgLkxZ6b3NWGLBCVjqUZA%40mail.gmail.com
---
 src/backend/replication/basebackup.c |  7 +++++
 src/backend/storage/file/fd.c        | 38 +++++++++++++++++++++++++---
 src/backend/storage/smgr/md.c        |  6 ++---
 src/include/storage/fd.h             |  1 +
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index e71d7406ed..7961a7041b 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -19,6 +19,7 @@
 #include "access/xlog_internal.h"	/* for pg_start/stop_backup */
 #include "catalog/pg_type.h"
 #include "common/file_perm.h"
+#include "common/string.h"
 #include "commands/progress.h"
 #include "lib/stringinfo.h"
 #include "libpq/libpq.h"
@@ -1268,6 +1269,12 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
 					strlen(PG_TEMP_FILE_PREFIX)) == 0)
 			continue;
 
+#ifdef WIN32
+		/* Skip unlinked files */
+		if (pg_str_endswith(de->d_name, ".unlinked"))
+			continue;
+#endif
+
 		/*
 		 * Check if the postmaster has signaled us to exit, and abort with an
 		 * error in that case. The error handler further up will call
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 501652654e..43f24d4ceb 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -72,6 +72,7 @@
 
 #include "postgres.h"
 
+#include <common/string.h>
 #include <dirent.h>
 #include <sys/file.h>
 #include <sys/param.h>
@@ -683,6 +684,35 @@ pg_truncate(const char *path, off_t length)
 #endif
 }
 
+/*
+ * Unlink a file.  On Windows, rename to a temporary filename before unlinking
+ * so that "path" is available for new files immediately even if other backends
+ * hold descriptors to the one we unlink.
+ */
+int
+pg_unlink(const char *path)
+{
+#ifdef WIN32
+	for (;;)
+	{
+		char		tmp_path[MAXPGPATH];
+
+		snprintf(tmp_path, sizeof(tmp_path), "%s.%ld.unlinked", path, random());
+		if (!MoveFileEx(path, tmp_path, 0))
+		{
+			if (GetLastError() == ERROR_FILE_EXISTS)
+				continue;		/* try a new random number */
+			_dosmaperr(GetLastError());
+			return -1;
+		}
+
+		return unlink(tmp_path);
+	}
+#else
+	return unlink(path);
+#endif
+}
+
 /*
  * fsync_fname -- fsync a file or directory, handling errors properly
  *
@@ -3269,8 +3299,9 @@ CleanupTempFiles(bool isCommit, bool isProcExit)
  * postmaster session
  *
  * This should be called during postmaster startup.  It will forcibly
- * remove any leftover files created by OpenTemporaryFile and any leftover
- * temporary relation files created by mdcreate.
+ * remove any leftover files created by OpenTemporaryFile, any leftover
+ * temporary relation files created by mdcreate, and anything left
+ * behind by pg_unlink() on crash.
  *
  * During post-backend-crash restart cycle, this routine is called when
  * remove_temp_files_after_crash GUC is enabled. Multiple crashes while
@@ -3448,7 +3479,8 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
 
 	while ((de = ReadDirExtended(dbspace_dir, dbspacedirname, LOG)) != NULL)
 	{
-		if (!looks_like_temp_rel_name(de->d_name))
+		if (!looks_like_temp_rel_name(de->d_name) &&
+			!pg_str_endswith(de->d_name, ".unlinked"))
 			continue;
 
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 8592d47e95..d519855d59 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -374,7 +374,7 @@ mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
 		/* Next unlink the file, unless it was already found to be missing */
 		if (ret == 0 || errno != ENOENT)
 		{
-			ret = unlink(path);
+			ret = pg_unlink(path);
 			if (ret < 0 && errno != ENOENT)
 				ereport(WARNING,
 						(errcode_for_file_access(),
@@ -422,7 +422,7 @@ mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
 				register_forget_request(rnode, forkNum, segno);
 			}
 
-			if (unlink(segpath) < 0)
+			if (pg_unlink(segpath) < 0)
 			{
 				/* ENOENT is expected after the last segment... */
 				if (errno != ENOENT)
@@ -1753,7 +1753,7 @@ mdunlinkfiletag(const FileTag *ftag, char *path)
 	pfree(p);
 
 	/* Try to unlink the file. */
-	return unlink(path);
+	return pg_unlink(path);
 }
 
 /*
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 329c7eba9a..be38f6a979 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -189,6 +189,7 @@ extern ssize_t pg_pwritev_with_retry(int fd,
 									 int iovcnt,
 									 off_t offset);
 extern int	pg_truncate(const char *path, off_t length);
+extern int	pg_unlink(const char *path);
 extern void fsync_fname(const char *fname, bool isdir);
 extern int	fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel);
 extern int	durable_rename(const char *oldfile, const char *newfile, int loglevel);
-- 
2.30.2