Ok, I've pushed the Windows patch. I'll watch the build farm to see
if I've broken any of the frankentoolchain Windows animals.
Mikael kindly upgraded conchuela, so that leaves just prairiedog
without fdatasync. I've attached a patch to drop the configure probe
for that once prairiedog's host is reassigned to new duties, if we're
agreed on that.
While in this part of the code I noticed another anachronism that
could be cleaned up: our handling of the old pre-standard BSD O_FSYNC
flag. Pulling on that I noticed I could remove a bunch of associated
macrology.
From b78c07a573c9f7e634eb9d33f57b07e9c37bfb47 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 20 Jul 2022 12:32:26 +1200
Subject: [PATCH 1/2] Remove O_FSYNC and associated macros.
O_FSYNC was a pre-standard way of spelling O_SYNC. It's not needed on
any modern system. We can just use O_SYNC directly, if it exists, and
get rid of our OPEN_SYNC_FLAG macro.
Likewise for O_DSYNC, we can just use it directly, if it exists, and get
rid of our OPEN_DATASYNC_FLAG macro. The only complication is that we
still want to avoid choosing open_datasync as a default if O_DSYNC has
the same value as O_SYNC, so there is no change in behavior.
---
src/backend/access/transam/xlog.c | 20 ++++++++++----------
src/bin/pg_test_fsync/pg_test_fsync.c | 12 ++++++------
src/include/access/xlogdefs.h | 19 +------------------
3 files changed, 17 insertions(+), 34 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 9854b51c6a..1da3b8eb2e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -171,10 +171,10 @@ const struct config_enum_entry sync_method_options[] = {
#ifdef HAVE_FDATASYNC
{"fdatasync", SYNC_METHOD_FDATASYNC, false},
#endif
-#ifdef OPEN_SYNC_FLAG
+#ifdef O_SYNC
{"open_sync", SYNC_METHOD_OPEN, false},
#endif
-#ifdef OPEN_DATASYNC_FLAG
+#ifdef O_DSYNC
{"open_datasync", SYNC_METHOD_OPEN_DSYNC, false},
#endif
{NULL, 0, false}
@@ -7894,10 +7894,10 @@ get_sync_bit(int method)
/*
* Optimize writes by bypassing kernel cache with O_DIRECT when using
- * O_SYNC/O_FSYNC and O_DSYNC. But only if archiving and streaming are
- * disabled, otherwise the archive command or walsender process will read
- * the WAL soon after writing it, which is guaranteed to cause a physical
- * read if we bypassed the kernel cache. We also skip the
+ * O_SYNC and O_DSYNC. But only if archiving and streaming are disabled,
+ * otherwise the archive command or walsender process will read the WAL
+ * soon after writing it, which is guaranteed to cause a physical read if
+ * we bypassed the kernel cache. We also skip the
* posix_fadvise(POSIX_FADV_DONTNEED) call in XLogFileClose() for the same
* reason.
*
@@ -7921,13 +7921,13 @@ get_sync_bit(int method)
case SYNC_METHOD_FSYNC_WRITETHROUGH:
case SYNC_METHOD_FDATASYNC:
return 0;
-#ifdef OPEN_SYNC_FLAG
+#ifdef O_SYNC
case SYNC_METHOD_OPEN:
- return OPEN_SYNC_FLAG | o_direct_flag;
+ return O_SYNC | o_direct_flag;
#endif
-#ifdef OPEN_DATASYNC_FLAG
+#ifdef O_DSYNC
case SYNC_METHOD_OPEN_DSYNC:
- return OPEN_DATASYNC_FLAG | o_direct_flag;
+ return O_DSYNC | o_direct_flag;
#endif
default:
/* can't happen (unless we are out of sync with option array) */
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index f7bc199a30..6739214eb8 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -300,7 +300,7 @@ test_sync(int writes_per_op)
printf(LABEL_FORMAT, "open_datasync");
fflush(stdout);
-#ifdef OPEN_DATASYNC_FLAG
+#ifdef O_DSYNC
if ((tmpfile = open_direct(filename, O_RDWR | O_DSYNC | PG_BINARY, 0)) == -1)
{
printf(NA_FORMAT, _("n/a*"));
@@ -407,8 +407,8 @@ test_sync(int writes_per_op)
printf(LABEL_FORMAT, "open_sync");
fflush(stdout);
-#ifdef OPEN_SYNC_FLAG
- if ((tmpfile = open_direct(filename, O_RDWR | OPEN_SYNC_FLAG | PG_BINARY, 0)) == -1)
+#ifdef O_SYNC
+ if ((tmpfile = open_direct(filename, O_RDWR | O_SYNC | PG_BINARY, 0)) == -1)
{
printf(NA_FORMAT, _("n/a*"));
fs_warning = true;
@@ -466,7 +466,7 @@ test_open_syncs(void)
static void
test_open_sync(const char *msg, int writes_size)
{
-#ifdef OPEN_SYNC_FLAG
+#ifdef O_SYNC
int tmpfile,
ops,
writes;
@@ -475,8 +475,8 @@ test_open_sync(const char *msg, int writes_size)
printf(LABEL_FORMAT, msg);
fflush(stdout);
-#ifdef OPEN_SYNC_FLAG
- if ((tmpfile = open_direct(filename, O_RDWR | OPEN_SYNC_FLAG | PG_BINARY, 0)) == -1)
+#ifdef O_SYNC
+ if ((tmpfile = open_direct(filename, O_RDWR | O_SYNC | PG_BINARY, 0)) == -1)
printf(NA_FORMAT, _("n/a*"));
else
{
diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h
index a47e3eeb1f..a720169b17 100644
--- a/src/include/access/xlogdefs.h
+++ b/src/include/access/xlogdefs.h
@@ -70,27 +70,10 @@ typedef uint16 RepOriginId;
* default method. We assume that fsync() is always available, and that
* configure determined whether fdatasync() is.
*/
-#if defined(O_SYNC)
-#define OPEN_SYNC_FLAG O_SYNC
-#elif defined(O_FSYNC)
-#define OPEN_SYNC_FLAG O_FSYNC
-#endif
-
-#if defined(O_DSYNC)
-#if defined(OPEN_SYNC_FLAG)
-/* O_DSYNC is distinct? */
-#if O_DSYNC != OPEN_SYNC_FLAG
-#define OPEN_DATASYNC_FLAG O_DSYNC
-#endif
-#else /* !defined(OPEN_SYNC_FLAG) */
-/* Win32 only has O_DSYNC */
-#define OPEN_DATASYNC_FLAG O_DSYNC
-#endif
-#endif
#if defined(PLATFORM_DEFAULT_SYNC_METHOD)
#define DEFAULT_SYNC_METHOD PLATFORM_DEFAULT_SYNC_METHOD
-#elif defined(OPEN_DATASYNC_FLAG)
+#elif defined(O_DSYNC) && O_DSYNC != O_SYNC
#define DEFAULT_SYNC_METHOD SYNC_METHOD_OPEN_DSYNC
#elif defined(HAVE_FDATASYNC)
#define DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
--
2.36.1
From 461e3591096933562f70112897e2a59dc5765190 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 20 Jul 2022 14:10:37 +1200
Subject: [PATCH 2/2] Remove fdatasync configure probe.
Every system we target has POSIX fdatasync, except Windows where we
provide a replacement in src/port/fdatasync.c. We can remove the
configure probe and associated #ifdefs.
We retain the probe for the function declaration, which allows us to
supply the missing declaration for macOS and Windows.
Discussion: https://postgr.es/m/CA%2BhUKGJZJVO%3DiX%2Beb-PXi2_XS9ZRqnn_4URh0NUQOwt6-_51xQ%40mail.gmail.com
---
configure | 59 +--------------------------
configure.ac | 3 --
src/backend/access/transam/xlog.c | 4 --
src/backend/storage/file/fd.c | 8 ----
src/bin/pg_test_fsync/pg_test_fsync.c | 4 --
src/include/access/xlogdefs.h | 4 +-
src/include/pg_config.h.in | 3 --
src/include/port/freebsd.h | 2 -
src/include/port/win32_port.h | 6 ---
src/tools/msvc/Solution.pm | 1 -
10 files changed, 2 insertions(+), 92 deletions(-)
diff --git a/configure b/configure
index 59fa82b8d7..6edee4af91 100755
--- a/configure
+++ b/configure
@@ -12315,63 +12315,6 @@ if test "$ac_res" != no; then :
fi
-# Solaris:
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing fdatasync" >&5
-$as_echo_n "checking for library containing fdatasync... " >&6; }
-if ${ac_cv_search_fdatasync+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- ac_func_search_save_LIBS=$LIBS
-cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-
-/* Override any GCC internal prototype to avoid an error.
- Use char because int might match the return type of a GCC
- builtin and then its argument prototype would still apply. */
-#ifdef __cplusplus
-extern "C"
-#endif
-char fdatasync ();
-int
-main ()
-{
-return fdatasync ();
- ;
- return 0;
-}
-_ACEOF
-for ac_lib in '' rt posix4; do
- if test -z "$ac_lib"; then
- ac_res="none required"
- else
- ac_res=-l$ac_lib
- LIBS="-l$ac_lib $ac_func_search_save_LIBS"
- fi
- if ac_fn_c_try_link "$LINENO"; then :
- ac_cv_search_fdatasync=$ac_res
-fi
-rm -f core conftest.err conftest.$ac_objext \
- conftest$ac_exeext
- if ${ac_cv_search_fdatasync+:} false; then :
- break
-fi
-done
-if ${ac_cv_search_fdatasync+:} false; then :
-
-else
- ac_cv_search_fdatasync=no
-fi
-rm conftest.$ac_ext
-LIBS=$ac_func_search_save_LIBS
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_fdatasync" >&5
-$as_echo "$ac_cv_search_fdatasync" >&6; }
-ac_res=$ac_cv_search_fdatasync
-if test "$ac_res" != no; then :
- test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
-
-fi
-
# Cygwin:
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing shmget" >&5
$as_echo_n "checking for library containing shmget... " >&6; }
@@ -16039,7 +15982,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in backtrace_symbols clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit inet_pton kqueue mbstowcs_l memset_s poll posix_fallocate ppoll pthread_is_threaded_np readlink readv setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink syncfs sync_file_range uselocale wcstombs_l writev
+for ac_func in backtrace_symbols clock_gettime copyfile getifaddrs getpeerucred getrlimit inet_pton kqueue mbstowcs_l memset_s poll posix_fallocate ppoll pthread_is_threaded_np readlink readv setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink syncfs sync_file_range uselocale wcstombs_l writev
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.ac b/configure.ac
index 612dabf698..177bee2507 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1253,8 +1253,6 @@ AC_SEARCH_LIBS(getopt_long, [getopt gnugetopt])
AC_SEARCH_LIBS(shm_open, rt)
AC_SEARCH_LIBS(shm_unlink, rt)
AC_SEARCH_LIBS(clock_gettime, [rt posix4])
-# Solaris:
-AC_SEARCH_LIBS(fdatasync, [rt posix4])
# Cygwin:
AC_SEARCH_LIBS(shmget, cygipc)
# *BSD:
@@ -1796,7 +1794,6 @@ AC_CHECK_FUNCS(m4_normalize([
backtrace_symbols
clock_gettime
copyfile
- fdatasync
getifaddrs
getpeerucred
getrlimit
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1da3b8eb2e..fc972873c8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -168,9 +168,7 @@ const struct config_enum_entry sync_method_options[] = {
#ifdef HAVE_FSYNC_WRITETHROUGH
{"fsync_writethrough", SYNC_METHOD_FSYNC_WRITETHROUGH, false},
#endif
-#ifdef HAVE_FDATASYNC
{"fdatasync", SYNC_METHOD_FDATASYNC, false},
-#endif
#ifdef O_SYNC
{"open_sync", SYNC_METHOD_OPEN, false},
#endif
@@ -8015,12 +8013,10 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
msg = _("could not fsync write-through file \"%s\": %m");
break;
#endif
-#ifdef HAVE_FDATASYNC
case SYNC_METHOD_FDATASYNC:
if (pg_fdatasync(fd) != 0)
msg = _("could not fdatasync file \"%s\": %m");
break;
-#endif
case SYNC_METHOD_OPEN:
case SYNC_METHOD_OPEN_DSYNC:
/* not reachable */
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index f904f60c08..c3169d14e6 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -442,20 +442,12 @@ pg_fsync_writethrough(int fd)
/*
* pg_fdatasync --- same as fdatasync except does nothing if enableFsync is off
- *
- * Not all platforms have fdatasync; treat as fsync if not available.
*/
int
pg_fdatasync(int fd)
{
if (enableFsync)
- {
-#ifdef HAVE_FDATASYNC
return fdatasync(fd);
-#else
- return fsync(fd);
-#endif
- }
else
return 0;
}
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index 6739214eb8..93785bba0a 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -331,7 +331,6 @@ test_sync(int writes_per_op)
printf(LABEL_FORMAT, "fdatasync");
fflush(stdout);
-#ifdef HAVE_FDATASYNC
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
die("could not open output file");
START_TIMER;
@@ -347,9 +346,6 @@ test_sync(int writes_per_op)
}
STOP_TIMER;
close(tmpfile);
-#else
- printf(NA_FORMAT, _("n/a"));
-#endif
/*
* Test fsync
diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h
index a720169b17..d2901e5270 100644
--- a/src/include/access/xlogdefs.h
+++ b/src/include/access/xlogdefs.h
@@ -75,10 +75,8 @@ typedef uint16 RepOriginId;
#define DEFAULT_SYNC_METHOD PLATFORM_DEFAULT_SYNC_METHOD
#elif defined(O_DSYNC) && O_DSYNC != O_SYNC
#define DEFAULT_SYNC_METHOD SYNC_METHOD_OPEN_DSYNC
-#elif defined(HAVE_FDATASYNC)
-#define DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
#else
-#define DEFAULT_SYNC_METHOD SYNC_METHOD_FSYNC
+#define DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
#endif
#endif /* XLOG_DEFS_H */
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 529fb84a86..7da7fb22e4 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -188,9 +188,6 @@
/* Define to 1 if you have the `explicit_bzero' function. */
#undef HAVE_EXPLICIT_BZERO
-/* Define to 1 if you have the `fdatasync' function. */
-#undef HAVE_FDATASYNC
-
/* Define to 1 if you have the `fls' function. */
#undef HAVE_FLS
diff --git a/src/include/port/freebsd.h b/src/include/port/freebsd.h
index 2e2e749a6b..0e3fde55d6 100644
--- a/src/include/port/freebsd.h
+++ b/src/include/port/freebsd.h
@@ -5,6 +5,4 @@
* would prefer open_datasync on FreeBSD 13+, but that is not a good choice on
* many systems.
*/
-#ifdef HAVE_FDATASYNC
#define PLATFORM_DEFAULT_SYNC_METHOD SYNC_METHOD_FDATASYNC
-#endif
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index 5ea66528fa..5121c0c626 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -83,12 +83,6 @@
#define HAVE_FSYNC_WRITETHROUGH
#define FSYNC_WRITETHROUGH_IS_FSYNC
-/*
- * We have a replacement for fdatasync() in src/port/fdatasync.c, which is
- * unconditionally used by MSVC and Mingw builds.
- */
-#define HAVE_FDATASYNC
-
#define USES_WINSOCK
/*
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 3ddcd024a7..49e2825974 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -257,7 +257,6 @@ sub GenerateFiles
HAVE_EDITLINE_READLINE_H => undef,
HAVE_EXECINFO_H => undef,
HAVE_EXPLICIT_BZERO => undef,
- HAVE_FDATASYNC => 1,
HAVE_FLS => undef,
HAVE_FSEEKO => 1,
HAVE_FUNCNAME__FUNC => undef,
--
2.36.1
