On Mon, Oct 09, 2023 at 01:12:16PM +0300, Maxim Orlov wrote: > On Fri, 6 Oct 2023 at 22:35, Nathan Bossart <nathandboss...@gmail.com> > wrote: >> From a quick skim, this one looks pretty good to me. Would you mind adding >> it to the commitfest so that it doesn't get lost? I will aim to take a >> closer look at it next week. > > Sounds good, thanks a lot! > > https://commitfest.postgresql.org/45/4609/
Thanks. I've made a couple of small changes, but otherwise I think this one is just about ready. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
>From 481efed2b47ab1cf37308a789698069d1a84561b Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Mon, 9 Oct 2023 11:07:29 -0500 Subject: [PATCH v12 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..f0ae034207 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 == 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
