Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qpsu8-007Q4x-2o for pgsql-hackers@arkaria.postgresql.org; Mon, 09 Oct 2023 16:14:48 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1qpsu6-00GT5t-2H for pgsql-hackers@arkaria.postgresql.org; Mon, 09 Oct 2023 16:14:46 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qpsu5-00GT5d-En for pgsql-hackers@lists.postgresql.org; Mon, 09 Oct 2023 16:14:46 +0000 Received: from mail-io1-xd36.google.com ([2607:f8b0:4864:20::d36]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1qpsu3-000oOP-BU for pgsql-hackers@postgresql.org; Mon, 09 Oct 2023 16:14:45 +0000 Received: by mail-io1-xd36.google.com with SMTP id ca18e2360f4ac-79f82b26abfso166653039f.1 for ; Mon, 09 Oct 2023 09:14:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696868082; x=1697472882; darn=postgresql.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ZLCF+G7p3ACYYu+MAE/Vrtv2fVU/Hd9diZU7sOFJy5Y=; b=GKFkiHXaiiIQDKiH4poxn6PP3ekAhHWnaueoBnhljpUL0WCoVtLWlL2ORuUxFYBw5S gmGt79ag73I8jts5SWQNGVRj0o0qqJBVpN7HQGvT3J65TEqBor/8lrGr59hxJvXd0fj5 TmTr+wtDzK30wIpklKnH8BRCik0GcZuUtEEK5GZWJ3r7CmkIgn1Pdrh2UrEFP9VY8Wsj S0ICN7MqQS7u5NQFTwdVVoYxUYQionjSsqjQ+9dwC07VL6g5W62331vXM/L59mu2Mqyw eC56dLW10A6DzU+FygwYcyZkS05xNHqCo46flksmT8nvxrBgrx4AYEcML/EkrQwBviHk OFtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696868082; x=1697472882; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ZLCF+G7p3ACYYu+MAE/Vrtv2fVU/Hd9diZU7sOFJy5Y=; b=cEH8TUVxUW+nTawzS8uwavyn9rx1jma2jFZEYyYup1RcZwEkCGIQ6lZUZm1ZqGqrRF B/oTWHre2dHwlhYlLR3cS4ATA0I8Lo9c/i2KYrUVWELrT7GacrbWSmE1/EXGv+dzqrbW RFSLJQKaouiLC/0kBAJs7vgiW1Fk7T7y+L2wGrHfjPR+b3082yPPl+XXpt8A/m5woXTb Bbe8WSq4lkBCUGI2XsMq8Xso9pdU1sLBcjphsM0P1kbXGWKn42tM3QncXVhPpLe8tlS4 6oSeU0Wjl3BiYJDQXkMjed/W0gb3dxB8eR0DrLvaohETOOm9PFNnc92izJZ4m+kaWTfs 3mSQ== X-Gm-Message-State: AOJu0YzyYNZAkuu7Afa5qNOc7ikoOG98kSOvCzMbsCxlJEpPEaoDkxRN KJoKgT7XrGKf0Vq0YSueVdg= X-Google-Smtp-Source: AGHT+IH8l+baZ0xI2+QjRFUvivUHg9KnRoP8BuELK4dl8jxf1e8UIvLbJp1yqRbpXe0vzhxPr9WGcw== X-Received: by 2002:a6b:f30b:0:b0:794:e015:585e with SMTP id m11-20020a6bf30b000000b00794e015585emr17157169ioh.21.1696868081830; Mon, 09 Oct 2023 09:14:41 -0700 (PDT) Received: from nathanxps13 (162-195-168-172.lightspeed.stlsmo.sbcglobal.net. [162.195.168.172]) by smtp.gmail.com with ESMTPSA id gk4-20020a0566386a8400b0042b451aa95esm2152931jab.150.2023.10.09.09.14.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Oct 2023 09:14:41 -0700 (PDT) Date: Mon, 9 Oct 2023 11:14:39 -0500 From: Nathan Bossart To: Maxim Orlov Cc: Peter Eisentraut , Michael Paquier , Justin Pryzby , Thomas Munro , David Steele , Fujii Masao , Bruce Momjian , Tom Lane , Michael Brown , pgsql-hackers@postgresql.org Subject: Re: should frontend tools use syncfs() ? Message-ID: <20231009161439.GA537266@nathanxps13> References: <20230801163707.GA862840@nathanxps13> <20230808200606.GA1384393@nathanxps13> <20230816151705.GB2651383@nathanxps13> <42804669-7063-1320-ed37-3226d5f1067d@eisentraut.org> <20231004152907.GD3178@nathanxps13> <20231006193544.GB155099@nathanxps13> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="ZPt4rx8FFjLCG7dd" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --ZPt4rx8FFjLCG7dd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Oct 09, 2023 at 01:12:16PM +0300, Maxim Orlov wrote: > On Fri, 6 Oct 2023 at 22:35, Nathan Bossart > 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 --ZPt4rx8FFjLCG7dd Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v12-0001-Align-names-of-variables-etc.-in-wal_sync_method.patch" From 481efed2b47ab1cf37308a789698069d1a84561b Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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 --ZPt4rx8FFjLCG7dd--