Hi,
We allow $SUBJECT on Windows. I'm not sure exactly how we finished up
with that, maybe a historical mistake, but I find it misleading today.
Modern Windows flushes drive write caches for fsync (= _commit()) and
fdatasync (= FLUSH_FLAGS_FILE_DATA_SYNC_ONLY). In fact it is possible
to tell Windows to write out file data without flushing the drive
cache (= FLUSH_FLAGS_NO_SYNC), but I don't believe anyone is
interested in new weaker levels. Any reason not to just get rid of
it?
On macOS, our fsync and fdatasync levels *don't* flush drive caches,
because those system calls don't on that OS, and they offer a weird
special fcntl, so there we offer $SUBJECT for a good reason. Now that
macOS 10.2 systems are thoroughly extinct, I think we might as well
drop the configure probe, though, while we're doing a lot of that sort
of thing.
The documentation also says a couple of things that aren't quite
correct about wal_sync_level. (I would also like to revise other
nearby outdated paragraphs about volatile write caches, sector sizes
etc, but that'll take some more research.)
From 42bd2d6c0ff4cd931ec3d4008438842bab5acd4b Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 26 Aug 2022 14:27:43 +1200
Subject: [PATCH 1/3] Remove wal_sync_method=fsync_writethrough on Windows.
The fsync and fdatasync levels already flush drive write caches on
Windows, so it only confuses matters to have an apparently higher level
that isn't actually higher. We don't do that on any other OS.
---
doc/src/sgml/wal.sgml | 5 ++---
src/backend/storage/file/fd.c | 6 ++----
src/bin/pg_test_fsync/pg_test_fsync.c | 4 +---
src/include/port/win32_port.h | 8 --------
4 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
index 01f7379ebb..90cbacfe41 100644
--- a/doc/src/sgml/wal.sgml
+++ b/doc/src/sgml/wal.sgml
@@ -108,9 +108,8 @@
<literal>open_datasync</literal> (the default), write caching can be disabled
by unchecking <literal>My Computer\Open\<replaceable>disk drive</replaceable>\Properties\Hardware\Properties\Policies\Enable write caching on the disk</literal>.
Alternatively, set <varname>wal_sync_method</varname> to
- <literal>fdatasync</literal> (NTFS only), <literal>fsync</literal> or
- <literal>fsync_writethrough</literal>, which prevent
- write caching.
+ <literal>fdatasync</literal> (NTFS only) or <literal>fsync</literal>,
+ which prevent write caching.
</para>
</listitem>
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index e3b19ca1ed..c27a6600fe 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -395,7 +395,7 @@ pg_fsync(int fd)
#endif
/* #if is to skip the sync_method test if there's no need for it */
-#if defined(HAVE_FSYNC_WRITETHROUGH) && !defined(FSYNC_WRITETHROUGH_IS_FSYNC)
+#if defined(HAVE_FSYNC_WRITETHROUGH)
if (sync_method == SYNC_METHOD_FSYNC_WRITETHROUGH)
return pg_fsync_writethrough(fd);
else
@@ -425,9 +425,7 @@ pg_fsync_writethrough(int fd)
{
if (enableFsync)
{
-#ifdef WIN32
- return _commit(fd);
-#elif defined(F_FULLFSYNC)
+#if defined(F_FULLFSYNC)
return (fcntl(fd, F_FULLFSYNC, 0) == -1) ? -1 : 0;
#else
errno = ENOSYS;
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index 77f0db0376..e23e2601ad 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -605,9 +605,7 @@ signal_cleanup(int signum)
static int
pg_fsync_writethrough(int fd)
{
-#ifdef WIN32
- return _commit(fd);
-#elif defined(F_FULLFSYNC)
+#if defined(F_FULLFSYNC)
return (fcntl(fd, F_FULLFSYNC, 0) == -1) ? -1 : 0;
#else
errno = ENOSYS;
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index 707f8760ca..b6769a1693 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -75,14 +75,6 @@
/* Windows doesn't have fsync() as such, use _commit() */
#define fsync(fd) _commit(fd)
-/*
- * For historical reasons, we allow setting wal_sync_method to
- * fsync_writethrough on Windows, even though it's really identical to fsync
- * (both code paths wind up at _commit()).
- */
-#define HAVE_FSYNC_WRITETHROUGH
-#define FSYNC_WRITETHROUGH_IS_FSYNC
-
#define USES_WINSOCK
/*
--
2.35.1
From 6e59fb92aafa4803ceed411e1610a4f458ed092b Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 26 Aug 2022 15:00:26 +1200
Subject: [PATCH 2/3] Remove configure probe for F_FULLSYNC.
This was done to avoid using it on very old macOS systems.
---
configure | 14 --------------
configure.ac | 3 ---
src/include/pg_config.h.in | 4 ----
src/include/port/darwin.h | 3 ---
src/tools/msvc/Solution.pm | 1 -
5 files changed, 25 deletions(-)
diff --git a/configure b/configure
index a268780c5d..6975c3f0ca 100755
--- a/configure
+++ b/configure
@@ -16204,20 +16204,6 @@ esac
fi
-# This is probably only present on macOS, but may as well check always
-ac_fn_c_check_decl "$LINENO" "F_FULLFSYNC" "ac_cv_have_decl_F_FULLFSYNC" "#include <fcntl.h>
-"
-if test "x$ac_cv_have_decl_F_FULLFSYNC" = xyes; then :
- ac_have_decl=1
-else
- ac_have_decl=0
-fi
-
-cat >>confdefs.h <<_ACEOF
-#define HAVE_DECL_F_FULLFSYNC $ac_have_decl
-_ACEOF
-
-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for PS_STRINGS" >&5
$as_echo_n "checking for PS_STRINGS... " >&6; }
if ${pgac_cv_var_PS_STRINGS+:} false; then :
diff --git a/configure.ac b/configure.ac
index 993b5d5cb0..e293bfdfb7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1798,9 +1798,6 @@ AC_CHECK_DECLS([strlcat, strlcpy, strnlen])
AC_CHECK_DECLS([preadv], [], [AC_LIBOBJ(preadv)], [#include <sys/uio.h>])
AC_CHECK_DECLS([pwritev], [], [AC_LIBOBJ(pwritev)], [#include <sys/uio.h>])
-# This is probably only present on macOS, but may as well check always
-AC_CHECK_DECLS(F_FULLFSYNC, [], [], [#include <fcntl.h>])
-
AC_CACHE_CHECK([for PS_STRINGS], [pgac_cv_var_PS_STRINGS],
[AC_LINK_IFELSE([AC_LANG_PROGRAM(
[#include <machine/vmparam.h>
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c5a80b829e..6d1e2c2904 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -95,10 +95,6 @@
don't. */
#undef HAVE_DECL_FDATASYNC
-/* Define to 1 if you have the declaration of `F_FULLFSYNC', and to 0 if you
- don't. */
-#undef HAVE_DECL_F_FULLFSYNC
-
/* Define to 1 if you have the declaration of
`LLVMCreateGDBRegistrationListener', and to 0 if you don't. */
#undef HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER
diff --git a/src/include/port/darwin.h b/src/include/port/darwin.h
index 15fb69d6db..36c54b2152 100644
--- a/src/include/port/darwin.h
+++ b/src/include/port/darwin.h
@@ -2,7 +2,4 @@
#define __darwin__ 1
-#if HAVE_DECL_F_FULLFSYNC /* not present before macOS 10.3 */
#define HAVE_FSYNC_WRITETHROUGH
-
-#endif
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index c2acb58df0..86dfa9faa6 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -233,7 +233,6 @@ sub GenerateFiles
HAVE_CRTDEFS_H => undef,
HAVE_CRYPTO_LOCK => undef,
HAVE_DECL_FDATASYNC => 0,
- HAVE_DECL_F_FULLFSYNC => 0,
HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER => 0,
HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER => 0,
HAVE_DECL_LLVMGETHOSTCPUNAME => 0,
--
2.35.1
From 136dd36dd2604daf0f7402e4fcd280415f61f005 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 26 Aug 2022 16:05:59 +1200
Subject: [PATCH 3/3] Doc: Corrections to claims about wal_sync_method.
We claimed that fsync_writethrough called fsync() and then flushed write
caches, but that is untrue. Describe the actual system call, and
mention that it's macOS-only.
We claimed that all the wal_sync_method values were equally reliable,
except that fsync_writethrough might flush write caches. In fact
open_[data]sync might differ from f[data]sync on several systems in
terms of power loss risk, depending on whether the filesystem tries to
use FUA, whether the driver passes it on, and whether the device honors
it. That's too technical a topic to go into here, but let's remove the
misleading statement.
We talked about "preventing caching" when we meant "flushing caches".
We said that open_[data]sync would get you O_DIRECT where available, but
that is not generally true. It is true only with wal_level=minimal and
max_wal_senders=0, which must surely be a very rare configuration.
---
doc/src/sgml/config.sgml | 3 +--
doc/src/sgml/wal.sgml | 10 ++++------
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a5cd4e44c7..eac0a1c8fe 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3061,7 +3061,7 @@ include_dir 'conf.d'
</listitem>
<listitem>
<para>
- <literal>fsync_writethrough</literal> (call <function>fsync()</function> at each commit, forcing write-through of any disk write cache)
+ <literal>fsync_writethrough</literal> (macOS only: call <function>fcntl(F_FULLSYNC)</function> at each commit, flushing any disk write cache)
</para>
</listitem>
<listitem>
@@ -3071,7 +3071,6 @@ include_dir 'conf.d'
</listitem>
</itemizedlist>
<para>
- The <literal>open_</literal>* options also use <literal>O_DIRECT</literal> if available.
Not all of these choices are available on all platforms.
The default is the first method in the above list that is supported
by the platform, except that <literal>fdatasync</literal> is the default on
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
index 90cbacfe41..cf5f4c41d9 100644
--- a/doc/src/sgml/wal.sgml
+++ b/doc/src/sgml/wal.sgml
@@ -109,13 +109,13 @@
by unchecking <literal>My Computer\Open\<replaceable>disk drive</replaceable>\Properties\Hardware\Properties\Policies\Enable write caching on the disk</literal>.
Alternatively, set <varname>wal_sync_method</varname> to
<literal>fdatasync</literal> (NTFS only) or <literal>fsync</literal>,
- which prevent write caching.
+ which flush write caches.
</para>
</listitem>
<listitem>
<para>
- On <productname>macOS</productname>, write caching can be prevented by
+ On <productname>macOS</productname>, write caches can be flushed by
setting <varname>wal_sync_method</varname> to <literal>fsync_writethrough</literal>.
</para>
</listitem>
@@ -756,10 +756,8 @@
The <xref linkend="guc-wal-sync-method"/> parameter determines how
<productname>PostgreSQL</productname> will ask the kernel to force
<acronym>WAL</acronym> updates out to disk.
- All the options should be the same in terms of reliability, with
- the exception of <literal>fsync_writethrough</literal>, which can sometimes
- force a flush of the disk cache even when other options do not do so.
- However, it's quite platform-specific which one will be the fastest.
+ It is platform-specific, and some cases also device-specific, which options
+ flush or write through volatile write caches.
You can test the speeds of different options using the <xref
linkend="pgtestfsync"/> program.
Note that this parameter is irrelevant if <varname>fsync</varname>
--
2.35.1
