pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nathan Bossart <nathandbossart@gmail.com>
To: Maxim Orlov <orlovmg@gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: David Steele <david@pgmasters.net>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Brown <michael.brown@discourse.org>
Cc: pgsql-hackers@postgresql.org
Subject: Re: should frontend tools use syncfs() ?
Date: Mon, 9 Oct 2023 14:34:27 -0500
Message-ID: <20231009193427.GA583396@nathanxps13> (raw )
In-Reply-To: <20231009161439.GA537266@nathanxps13 >
References: <20230808200606.GA1384393@nathanxps13 >
<ZNwF0u/89JFUOQiM@paquier.xyz >
<20230816151705.GB2651383@nathanxps13 >
<ZN2ZB4afQ2JbR9TA@paquier.xyz >
<42804669-7063-1320-ed37-3226d5f1067d@eisentraut.org >
<20231004152907.GD3178@nathanxps13 >
<CACG=ezaiHR1CU6zOc0xD_sJdZ8JiQKMLJFohnmyrW4dUR09QPA@mail.gmail.com >
<20231006193544.GB155099@nathanxps13 >
<CACG=ezY_K9jbRwJhj4_aEomWhrioh_T8TosJpenjtx3jpB2MKw@mail.gmail.com >
<20231009161439.GA537266@nathanxps13 >
On Mon, Oct 09, 2023 at 11:14:39AM -0500, Nathan Bossart wrote:
> Thanks. I've made a couple of small changes, but otherwise I think this
> one is just about ready.
I forgot to rename one thing. Here's a v13 with that fixed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v13-0001-Align-names-of-variables-etc.-in-wal_sync_method.patch (11.5K, ../20231009193427.GA583396@nathanxps13/2-v13-0001-Align-names-of-variables-etc.-in-wal_sync_method.patch)
download | inline diff:
From b1829fea8f81cceceabd37585ab74a290505d0d5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 9 Oct 2023 11:07:29 -0500
Subject: [PATCH v13 1/1] Align names of variables, etc. in
wal_sync_method-related code.
Author: Maxim Orlov
Discussion: https://postgr.es/m/CACG%3DezbL1gwE7_K7sr9uqaCGkWhmvRTcTEnm3%2BX1xsRNwbXULQ%40mail.gmail.com
---
src/backend/access/transam/xlog.c | 58 ++++++++++++++---------------
src/backend/storage/file/fd.c | 4 +-
src/backend/utils/misc/guc_tables.c | 8 ++--
src/include/access/xlog.h | 15 +++++---
src/include/access/xlogdefs.h | 8 ++--
src/include/port/freebsd.h | 2 +-
src/include/port/linux.h | 2 +-
src/include/utils/guc_hooks.h | 2 +-
src/tools/pgindent/typedefs.list | 1 +
9 files changed, 52 insertions(+), 48 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fcbde10529..a0acd67772 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -130,7 +130,7 @@ bool *wal_consistency_checking = NULL;
bool wal_init_zero = true;
bool wal_recycle = true;
bool log_checkpoints = true;
-int sync_method = DEFAULT_SYNC_METHOD;
+int wal_sync_method = DEFAULT_WAL_SYNC_METHOD;
int wal_level = WAL_LEVEL_REPLICA;
int CommitDelay = 0; /* precommit delay in microseconds */
int CommitSiblings = 5; /* # concurrent xacts needed to sleep */
@@ -171,17 +171,17 @@ static bool check_wal_consistency_checking_deferred = false;
/*
* GUC support
*/
-const struct config_enum_entry sync_method_options[] = {
- {"fsync", SYNC_METHOD_FSYNC, false},
+const struct config_enum_entry wal_sync_method_options[] = {
+ {"fsync", WAL_SYNC_METHOD_FSYNC, false},
#ifdef HAVE_FSYNC_WRITETHROUGH
- {"fsync_writethrough", SYNC_METHOD_FSYNC_WRITETHROUGH, false},
+ {"fsync_writethrough", WAL_SYNC_METHOD_FSYNC_WRITETHROUGH, false},
#endif
- {"fdatasync", SYNC_METHOD_FDATASYNC, false},
+ {"fdatasync", WAL_SYNC_METHOD_FDATASYNC, false},
#ifdef O_SYNC
- {"open_sync", SYNC_METHOD_OPEN, false},
+ {"open_sync", WAL_SYNC_METHOD_OPEN, false},
#endif
#ifdef O_DSYNC
- {"open_datasync", SYNC_METHOD_OPEN_DSYNC, false},
+ {"open_datasync", WAL_SYNC_METHOD_OPEN_DSYNC, false},
#endif
{NULL, 0, false}
};
@@ -2328,8 +2328,8 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
* have no open file or the wrong one. However, we do not need to
* fsync more than one file.
*/
- if (sync_method != SYNC_METHOD_OPEN &&
- sync_method != SYNC_METHOD_OPEN_DSYNC)
+ if (wal_sync_method != WAL_SYNC_METHOD_OPEN &&
+ wal_sync_method != WAL_SYNC_METHOD_OPEN_DSYNC)
{
if (openLogFile >= 0 &&
!XLByteInPrevSeg(LogwrtResult.Write, openLogSegNo,
@@ -2959,7 +2959,7 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli,
*/
*added = false;
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | O_CLOEXEC |
- get_sync_bit(sync_method));
+ get_sync_bit(wal_sync_method));
if (fd < 0)
{
if (errno != ENOENT)
@@ -3124,7 +3124,7 @@ XLogFileInit(XLogSegNo logsegno, TimeLineID logtli)
/* Now open original target segment (might not be file I just made) */
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | O_CLOEXEC |
- get_sync_bit(sync_method));
+ get_sync_bit(wal_sync_method));
if (fd < 0)
ereport(ERROR,
(errcode_for_file_access(),
@@ -3356,7 +3356,7 @@ XLogFileOpen(XLogSegNo segno, TimeLineID tli)
XLogFilePath(path, tli, segno, wal_segment_size);
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | O_CLOEXEC |
- get_sync_bit(sync_method));
+ get_sync_bit(wal_sync_method));
if (fd < 0)
ereport(PANIC,
(errcode_for_file_access(),
@@ -8118,16 +8118,16 @@ get_sync_bit(int method)
* not included in the enum option array, and therefore will never
* be seen here.
*/
- case SYNC_METHOD_FSYNC:
- case SYNC_METHOD_FSYNC_WRITETHROUGH:
- case SYNC_METHOD_FDATASYNC:
+ case WAL_SYNC_METHOD_FSYNC:
+ case WAL_SYNC_METHOD_FSYNC_WRITETHROUGH:
+ case WAL_SYNC_METHOD_FDATASYNC:
return o_direct_flag;
#ifdef O_SYNC
- case SYNC_METHOD_OPEN:
+ case WAL_SYNC_METHOD_OPEN:
return O_SYNC | o_direct_flag;
#endif
#ifdef O_DSYNC
- case SYNC_METHOD_OPEN_DSYNC:
+ case WAL_SYNC_METHOD_OPEN_DSYNC:
return O_DSYNC | o_direct_flag;
#endif
default:
@@ -8141,9 +8141,9 @@ get_sync_bit(int method)
* GUC support
*/
void
-assign_xlog_sync_method(int new_sync_method, void *extra)
+assign_wal_sync_method(int new_wal_sync_method, void *extra)
{
- if (sync_method != new_sync_method)
+ if (wal_sync_method != new_wal_sync_method)
{
/*
* To ensure that no blocks escape unsynced, force an fsync on the
@@ -8169,7 +8169,7 @@ assign_xlog_sync_method(int new_sync_method, void *extra)
}
pgstat_report_wait_end();
- if (get_sync_bit(sync_method) != get_sync_bit(new_sync_method))
+ if (get_sync_bit(wal_sync_method) != get_sync_bit(new_wal_sync_method))
XLogFileClose();
}
}
@@ -8195,8 +8195,8 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
* file.
*/
if (!enableFsync ||
- sync_method == SYNC_METHOD_OPEN ||
- sync_method == SYNC_METHOD_OPEN_DSYNC)
+ wal_sync_method == WAL_SYNC_METHOD_OPEN ||
+ wal_sync_method == WAL_SYNC_METHOD_OPEN_DSYNC)
return;
/* Measure I/O timing to sync the WAL file */
@@ -8206,29 +8206,29 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
INSTR_TIME_SET_ZERO(start);
pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC);
- switch (sync_method)
+ switch (wal_sync_method)
{
- case SYNC_METHOD_FSYNC:
+ case WAL_SYNC_METHOD_FSYNC:
if (pg_fsync_no_writethrough(fd) != 0)
msg = _("could not fsync file \"%s\": %m");
break;
#ifdef HAVE_FSYNC_WRITETHROUGH
- case SYNC_METHOD_FSYNC_WRITETHROUGH:
+ case WAL_SYNC_METHOD_FSYNC_WRITETHROUGH:
if (pg_fsync_writethrough(fd) != 0)
msg = _("could not fsync write-through file \"%s\": %m");
break;
#endif
- case SYNC_METHOD_FDATASYNC:
+ case WAL_SYNC_METHOD_FDATASYNC:
if (pg_fdatasync(fd) != 0)
msg = _("could not fdatasync file \"%s\": %m");
break;
- case SYNC_METHOD_OPEN:
- case SYNC_METHOD_OPEN_DSYNC:
+ case WAL_SYNC_METHOD_OPEN:
+ case WAL_SYNC_METHOD_OPEN_DSYNC:
/* not reachable */
Assert(false);
break;
default:
- elog(PANIC, "unrecognized wal_sync_method: %d", sync_method);
+ elog(PANIC, "unrecognized wal_sync_method: %d", wal_sync_method);
break;
}
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 3fed475c38..b7607aa1be 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -398,9 +398,9 @@ pg_fsync(int fd)
errno = 0;
#endif
- /* #if is to skip the sync_method test if there's no need for it */
+ /* #if is to skip the wal_sync_method test if there's no need for it */
#if defined(HAVE_FSYNC_WRITETHROUGH)
- if (sync_method == SYNC_METHOD_FSYNC_WRITETHROUGH)
+ if (wal_sync_method == WAL_SYNC_METHOD_FSYNC_WRITETHROUGH)
return pg_fsync_writethrough(fd);
else
#endif
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 16ec6c5ef0..4c58574166 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -485,7 +485,7 @@ static const struct config_enum_entry wal_compression_options[] = {
extern const struct config_enum_entry wal_level_options[];
extern const struct config_enum_entry archive_mode_options[];
extern const struct config_enum_entry recovery_target_action_options[];
-extern const struct config_enum_entry sync_method_options[];
+extern const struct config_enum_entry wal_sync_method_options[];
extern const struct config_enum_entry dynamic_shared_memory_options[];
/*
@@ -4843,9 +4843,9 @@ struct config_enum ConfigureNamesEnum[] =
gettext_noop("Selects the method used for forcing WAL updates to disk."),
NULL
},
- &sync_method,
- DEFAULT_SYNC_METHOD, sync_method_options,
- NULL, assign_xlog_sync_method, NULL
+ &wal_sync_method,
+ DEFAULT_WAL_SYNC_METHOD, wal_sync_method_options,
+ NULL, assign_wal_sync_method, NULL
},
{
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 48ca852381..4ad572cb87 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -19,12 +19,15 @@
/* Sync methods */
-#define SYNC_METHOD_FSYNC 0
-#define SYNC_METHOD_FDATASYNC 1
-#define SYNC_METHOD_OPEN 2 /* for O_SYNC */
-#define SYNC_METHOD_FSYNC_WRITETHROUGH 3
-#define SYNC_METHOD_OPEN_DSYNC 4 /* for O_DSYNC */
-extern PGDLLIMPORT int sync_method;
+typedef enum WalSyncMethod
+{
+ WAL_SYNC_METHOD_FSYNC = 0,
+ WAL_SYNC_METHOD_FDATASYNC,
+ WAL_SYNC_METHOD_OPEN, /* for O_SYNC */
+ WAL_SYNC_METHOD_FSYNC_WRITETHROUGH,
+ WAL_SYNC_METHOD_OPEN_DSYNC /* for O_DSYNC */
+} WalSyncMethod;
+extern PGDLLIMPORT int wal_sync_method;
extern PGDLLIMPORT XLogRecPtr ProcLastRecPtr;
extern PGDLLIMPORT XLogRecPtr XactLastRecEnd;
diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h
index fe794c7740..5f54c2816b 100644
--- a/src/include/access/xlogdefs.h
+++ b/src/include/access/xlogdefs.h
@@ -71,12 +71,12 @@ typedef uint16 RepOriginId;
*
* Note that we define our own O_DSYNC on Windows, but not O_SYNC.
*/
-#if defined(PLATFORM_DEFAULT_SYNC_METHOD)
-#define DEFAULT_SYNC_METHOD PLATFORM_DEFAULT_SYNC_METHOD
+#if defined(PLATFORM_DEFAULT_WAL_SYNC_METHOD)
+#define DEFAULT_WAL_SYNC_METHOD PLATFORM_DEFAULT_WAL_SYNC_METHOD
#elif defined(O_DSYNC) && (!defined(O_SYNC) || O_DSYNC != O_SYNC)
-#define DEFAULT_SYNC_METHOD SYNC_METHOD_OPEN_DSYNC
+#define DEFAULT_WAL_SYNC_METHOD WAL_SYNC_METHOD_OPEN_DSYNC
#else
-#define DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
+#define DEFAULT_WAL_SYNC_METHOD WAL_SYNC_METHOD_FDATASYNC
#endif
#endif /* XLOG_DEFS_H */
diff --git a/src/include/port/freebsd.h b/src/include/port/freebsd.h
index 0e3fde55d6..c604187acd 100644
--- a/src/include/port/freebsd.h
+++ b/src/include/port/freebsd.h
@@ -5,4 +5,4 @@
* would prefer open_datasync on FreeBSD 13+, but that is not a good choice on
* many systems.
*/
-#define PLATFORM_DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
+#define PLATFORM_DEFAULT_WAL_SYNC_METHOD WAL_SYNC_METHOD_FDATASYNC
diff --git a/src/include/port/linux.h b/src/include/port/linux.h
index 7a6e46cdbb..8101af2b93 100644
--- a/src/include/port/linux.h
+++ b/src/include/port/linux.h
@@ -19,4 +19,4 @@
* perform better and (b) causes outright failures on ext4 data=journal
* filesystems, because those don't support O_DIRECT.
*/
-#define PLATFORM_DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
+#define PLATFORM_DEFAULT_WAL_SYNC_METHOD WAL_SYNC_METHOD_FDATASYNC
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index f04b99e3b9..2a191830a8 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -159,6 +159,6 @@ extern bool check_wal_consistency_checking(char **newval, void **extra,
GucSource source);
extern void assign_wal_consistency_checking(const char *newval, void *extra);
extern bool check_wal_segment_size(int *newval, void **extra, GucSource source);
-extern void assign_xlog_sync_method(int new_sync_method, void *extra);
+extern void assign_wal_sync_method(int new_wal_sync_method, void *extra);
#endif /* GUC_HOOKS_H */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 8de90c4958..e69bb671bf 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3016,6 +3016,7 @@ WalSnd
WalSndCtlData
WalSndSendDataCallback
WalSndState
+WalSyncMethod
WalTimeSample
WalUsage
WalWriteMethod
--
2.25.1
view thread (50+ messages) latest in thread
Message-ID: <20231009193427.GA583396@nathanxps13>
Permalink: ../20231009193427.GA583396@nathanxps13/
Also on: postgresql.org/message-id/20231009193427.GA583396@nathanxps13
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, orlovmg@gmail.com, peter@eisentraut.org, michael@paquier.xyz, pryzby@telsasoft.com, thomas.munro@gmail.com, david@pgmasters.net, masao.fujii@oss.nttdata.com, bruce@momjian.us, tgl@sss.pgh.pa.us, michael.brown@discourse.org
Subject: Re: should frontend tools use syncfs() ?
In-Reply-To: <20231009193427.GA583396@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox