From a452b239e3b0815f80ef1138634e158357e30126 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 16 Jul 2020 15:53:09 +0900 Subject: [PATCH] Switch pg_test_fsync to use binary mode on Windows pg_test_fsync has always opened files using the text mode on Windows, as this is the default mode used if not enforced by _setmode(). This fixes a failure when running pg_test_fsync down to 12 because O_DSYNC and the text mode are not able to work together nicely. We fixed the handling of O_DSYNC in 12~ for the tool by switching to the concurrent-safe version of fopen() in src/port/ with 0ba06e0. And 40cfe86, by enforcing the text mode for compatibility reasons if O_TEXT or O_BINARY are not specified by the caller, broke pg_test_fsync. For all versions, this avoids any translation overhead, and pg_test_fsync should test binary writes, so it is a gain in all cases. Note that O_DSYNC is still not handled correctly in ~11, leading to pg_test_fsync to show insanely high numbers for open_datasync() (using this property it is easy to notice that the binary mode is much faster). This would require a backpatch of 0ba06e0 and 40cfe86, which could potentially break existing applications, so this is left out. There are no TAP tests for this tool yet, so I have checked all builds manually using MSVC. We could invent a new option to run a single transaction instead of using a duration of 1s to make the tests a maximum short, but this is left as future work. Thanks to Bruce Momjian for the discussion. Reported-by: Jeff Janes Author: Michael Paquier Discussion: https://postgr.es/m/16526-279ded30a230d275@postgresql.org Backpatch-through: 9.5 --- src/bin/pg_test_fsync/pg_test_fsync.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c index b4f3551b94a..5161094d129 100644 --- a/src/bin/pg_test_fsync/pg_test_fsync.c +++ b/src/bin/pg_test_fsync/pg_test_fsync.c @@ -218,7 +218,7 @@ test_open(void) /* * test if we can open the target file */ - if ((tmpfile = open(filename, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) == -1) + if ((tmpfile = open(filename, O_RDWR | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR)) == -1) die("could not open output file"); needs_unlink = 1; if (write(tmpfile, full_buf, XLOG_SEG_SIZE) != XLOG_SEG_SIZE) @@ -252,7 +252,7 @@ test_sync(int writes_per_op) fflush(stdout); #ifdef OPEN_DATASYNC_FLAG - if ((tmpfile = open(filename, O_RDWR | O_DSYNC | PG_O_DIRECT, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | O_DSYNC | PG_O_DIRECT | PG_BINARY, 0)) == -1) { printf(NA_FORMAT, "n/a*\n"); fs_warning = true; @@ -282,7 +282,7 @@ test_sync(int writes_per_op) fflush(stdout); #ifdef HAVE_FDATASYNC - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); START_TIMER; for (ops = 0; alarm_triggered == false; ops++) @@ -306,7 +306,7 @@ test_sync(int writes_per_op) printf(LABEL_FORMAT, "fsync"); fflush(stdout); - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); START_TIMER; for (ops = 0; alarm_triggered == false; ops++) @@ -329,7 +329,7 @@ test_sync(int writes_per_op) fflush(stdout); #ifdef HAVE_FSYNC_WRITETHROUGH - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); START_TIMER; for (ops = 0; alarm_triggered == false; ops++) @@ -355,7 +355,7 @@ test_sync(int writes_per_op) fflush(stdout); #ifdef OPEN_SYNC_FLAG - if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT | PG_BINARY, 0)) == -1) { printf(NA_FORMAT, "n/a*\n"); fs_warning = true; @@ -422,7 +422,7 @@ test_open_sync(const char *msg, int writes_size) fflush(stdout); #ifdef OPEN_SYNC_FLAG - if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT | PG_BINARY, 0)) == -1) printf(NA_FORMAT, "n/a*\n"); else { @@ -470,7 +470,7 @@ test_file_descriptor_sync(void) START_TIMER; for (ops = 0; alarm_triggered == false; ops++) { - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); @@ -482,7 +482,7 @@ test_file_descriptor_sync(void) * open and close the file again to be consistent with the following * test */ - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); close(tmpfile); } @@ -498,13 +498,13 @@ test_file_descriptor_sync(void) START_TIMER; for (ops = 0; alarm_triggered == false; ops++) { - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); close(tmpfile); /* reopen file */ - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); if (fsync(tmpfile) != 0) die("fsync failed"); @@ -529,7 +529,7 @@ test_non_sync(void) START_TIMER; for (ops = 0; alarm_triggered == false; ops++) { - if ((tmpfile = open(filename, O_RDWR, 0)) == -1) + if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) die("could not open output file"); if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); -- 2.39.5