Date: Mon, 21 Aug 2023 19:06:32 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	Thomas Munro <thomas.munro@gmail.com>,
	David Steele <david@pgmasters.net>,
	Fujii Masao <masao.fujii@oss.nttdata.com>,
	Bruce Momjian <bruce@momjian.us>, Tom Lane <tgl@sss.pgh.pa.us>,
	Michael Brown <michael.brown@discourse.org>,
	pgsql-hackers@postgresql.org
Subject: Re: should frontend tools use syncfs() ?
Message-ID: <20230822020632.GA565843@nathanxps13>
References: <20230731183946.GB537853@nathanxps13>
 <20230801163707.GA862840@nathanxps13>
 <20230808200606.GA1384393@nathanxps13>
 <ZNwF0u/89JFUOQiM@paquier.xyz>
 <20230816151705.GB2651383@nathanxps13>
 <ZN2ZB4afQ2JbR9TA@paquier.xyz>
 <20230818160111.GA189711@nathanxps13>
 <ZOP5qoUualu5xl2Z@paquier.xyz>
 <20230822014407.GA449131@nathanxps13>
 <ZOQUSbi8bD1nhxb4@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="cNdxnHkX5QqsyA0e"
Content-Disposition: inline
In-Reply-To: <ZOQUSbi8bD1nhxb4@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/20230822020632.GA565843%40nathanxps13>
Precedence: bulk


--cNdxnHkX5QqsyA0e
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Aug 22, 2023 at 10:50:01AM +0900, Michael Paquier wrote:
> On Mon, Aug 21, 2023 at 06:44:07PM -0700, Nathan Bossart wrote:
>> I'm hoping there's a simpler path forward here.  pg_rewind only needs the
>> following lines from fd.h:
>> 
>> 	/* Filename components */
>> 	#define PG_TEMP_FILES_DIR "pgsql_tmp"
>> 	#define PG_TEMP_FILE_PREFIX "pgsql_tmp"
>> 
>> Maybe we could move these to file_utils.h instead.  WDYT?
> 
> I guess so..  At the same time, something can be said about
> pg_checksums that redeclares PG_TEMP_FILE_PREFIX and PG_TEMP_FILES_DIR
> because it does not want to include fd.h and its sync routines.

This would look something like the attached patch.  I think this is nicer.
With this patch, we don't have to choose between including fd.h or
redefining the macros in the frontend code.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

--cNdxnHkX5QqsyA0e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="0001-move-PG_TEMP_FILE-macros-to-file_utils.h.patch"

From af40f64933b5d8829b7e1641ce57e4ef8fd5a2c2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 21 Aug 2023 19:05:14 -0700
Subject: [PATCH 1/1] move PG_TEMP_FILE* macros to file_utils.h

---
 src/backend/backup/basebackup.c     |  2 +-
 src/backend/storage/file/fileset.c  |  1 +
 src/bin/pg_checksums/pg_checksums.c | 10 ----------
 src/bin/pg_rewind/filemap.c         |  2 +-
 src/include/common/file_utils.h     |  4 ++++
 src/include/storage/fd.h            |  4 ----
 6 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 45be21131c..5d66014499 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -25,6 +25,7 @@
 #include "commands/defrem.h"
 #include "common/compression.h"
 #include "common/file_perm.h"
+#include "common/file_utils.h"
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "nodes/pg_list.h"
@@ -37,7 +38,6 @@
 #include "storage/bufpage.h"
 #include "storage/checksum.h"
 #include "storage/dsm_impl.h"
-#include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/reinit.h"
 #include "utils/builtins.h"
diff --git a/src/backend/storage/file/fileset.c b/src/backend/storage/file/fileset.c
index e9951b0269..84cae32548 100644
--- a/src/backend/storage/file/fileset.c
+++ b/src/backend/storage/file/fileset.c
@@ -25,6 +25,7 @@
 
 #include "catalog/pg_tablespace.h"
 #include "commands/tablespace.h"
+#include "common/file_utils.h"
 #include "common/hashfn.h"
 #include "miscadmin.h"
 #include "storage/ipc.h"
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 19eb67e485..9011a19b4e 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -52,16 +52,6 @@ typedef enum
 	PG_MODE_ENABLE
 } PgChecksumMode;
 
-/*
- * Filename components.
- *
- * XXX: fd.h is not declared here as frontend side code is not able to
- * interact with the backend-side definitions for the various fsync
- * wrappers.
- */
-#define PG_TEMP_FILES_DIR "pgsql_tmp"
-#define PG_TEMP_FILE_PREFIX "pgsql_tmp"
-
 static PgChecksumMode mode = PG_MODE_CHECK;
 
 static const char *progname;
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index bd5c598e20..58280d9abc 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -27,12 +27,12 @@
 #include <unistd.h>
 
 #include "catalog/pg_tablespace_d.h"
+#include "common/file_utils.h"
 #include "common/hashfn.h"
 #include "common/string.h"
 #include "datapagemap.h"
 #include "filemap.h"
 #include "pg_rewind.h"
-#include "storage/fd.h"
 
 /*
  * Define a hash table which we can use to store information about the files
diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h
index b7efa1226d..dd1532bcb0 100644
--- a/src/include/common/file_utils.h
+++ b/src/include/common/file_utils.h
@@ -46,4 +46,8 @@ extern ssize_t pg_pwritev_with_retry(int fd,
 
 extern ssize_t pg_pwrite_zeros(int fd, size_t size, off_t offset);
 
+/* Filename components */
+#define PG_TEMP_FILES_DIR "pgsql_tmp"
+#define PG_TEMP_FILE_PREFIX "pgsql_tmp"
+
 #endif							/* FILE_UTILS_H */
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 6791a406fc..aea30c0622 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -195,8 +195,4 @@ extern int	durable_unlink(const char *fname, int elevel);
 extern void SyncDataDirectory(void);
 extern int	data_sync_elevel(int elevel);
 
-/* Filename components */
-#define PG_TEMP_FILES_DIR "pgsql_tmp"
-#define PG_TEMP_FILE_PREFIX "pgsql_tmp"
-
 #endif							/* FD_H */
-- 
2.25.1


--cNdxnHkX5QqsyA0e--