On Wed, Jul 20, 2022 at 4:14 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Wed, Jul 20, 2022 at 4:08 PM David Rowley <dgrowle...@gmail.com> wrote:
> > On Wed, 20 Jul 2022 at 15:22, Thomas Munro <thomas.mu...@gmail.com> wrote:
> > > 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.
> >
> > Just to get in there before the farm does... I just got a boatload of
> > redefinition of HAVE_FDATASYNC warnings. I see it already gets
> > defined in pg_config.h
> >
> > All compiles cleanly with the attached.
>
> Oops. Thanks, pushed.
... and here's a rebase of the patch that removes that macro stuff,
since cfbot is watching this thread.
From c0c3e67637abf67d9b946a08ddc5c597b37b40a1 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 v2 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 b9067bfe81678376acd1b7779fbc68fbff85ed05 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 v2 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 | 8 ----
src/tools/msvc/Solution.pm | 1 -
10 files changed, 2 insertions(+), 94 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 4de5bf3bf6..5121c0c626 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -83,14 +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.
- */
-#ifndef HAVE_FDATASYNC
-#define HAVE_FDATASYNC
-#endif
-
#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
