Skip to content

Commit de559c2

Browse files
michaelpqmichaelpq
committed
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
1 parent c6d43ff commit de559c2

File tree

1 file changed

+12
-12
lines changed

1 file changed

+12
-12
lines changed

‎src/bin/pg_test_fsync/pg_test_fsync.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ test_open(void)
223223
/*
224224
* test if we can open the target file
225225
*/
226-
if ((tmpfile = open(filename, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) == -1)
226+
if ((tmpfile = open(filename, O_RDWR | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR)) == -1)
227227
die("could not open output file");
228228
needs_unlink = 1;
229229
if (write(tmpfile, full_buf, DEFAULT_XLOG_SEG_SIZE) !=
@@ -258,7 +258,7 @@ test_sync(int writes_per_op)
258258
fflush(stdout);
259259

260260
#ifdef OPEN_DATASYNC_FLAG
261-
if ((tmpfile = open(filename, O_RDWR | O_DSYNC | PG_O_DIRECT, 0)) == -1)
261+
if ((tmpfile = open(filename, O_RDWR | O_DSYNC | PG_O_DIRECT | PG_BINARY, 0)) == -1)
262262
{
263263
printf(NA_FORMAT, _("n/a*"));
264264
fs_warning = true;
@@ -288,7 +288,7 @@ test_sync(int writes_per_op)
288288
fflush(stdout);
289289

290290
#ifdef HAVE_FDATASYNC
291-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
291+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
292292
die("could not open output file");
293293
START_TIMER;
294294
for (ops = 0; alarm_triggered == false; ops++)
@@ -312,7 +312,7 @@ test_sync(int writes_per_op)
312312
printf(LABEL_FORMAT, "fsync");
313313
fflush(stdout);
314314

315-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
315+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
316316
die("could not open output file");
317317
START_TIMER;
318318
for (ops = 0; alarm_triggered == false; ops++)
@@ -335,7 +335,7 @@ test_sync(int writes_per_op)
335335
fflush(stdout);
336336

337337
#ifdef HAVE_FSYNC_WRITETHROUGH
338-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
338+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
339339
die("could not open output file");
340340
START_TIMER;
341341
for (ops = 0; alarm_triggered == false; ops++)
@@ -361,7 +361,7 @@ test_sync(int writes_per_op)
361361
fflush(stdout);
362362

363363
#ifdef OPEN_SYNC_FLAG
364-
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT, 0)) == -1)
364+
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT | PG_BINARY, 0)) == -1)
365365
{
366366
printf(NA_FORMAT, _("n/a*"));
367367
fs_warning = true;
@@ -428,7 +428,7 @@ test_open_sync(const char *msg, int writes_size)
428428
fflush(stdout);
429429

430430
#ifdef OPEN_SYNC_FLAG
431-
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT, 0)) == -1)
431+
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT | PG_BINARY, 0)) == -1)
432432
printf(NA_FORMAT, _("n/a*"));
433433
else
434434
{
@@ -476,7 +476,7 @@ test_file_descriptor_sync(void)
476476
START_TIMER;
477477
for (ops = 0; alarm_triggered == false; ops++)
478478
{
479-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
479+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
480480
die("could not open output file");
481481
if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
482482
die("write failed");
@@ -488,7 +488,7 @@ test_file_descriptor_sync(void)
488488
* open and close the file again to be consistent with the following
489489
* test
490490
*/
491-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
491+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
492492
die("could not open output file");
493493
close(tmpfile);
494494
}
@@ -504,13 +504,13 @@ test_file_descriptor_sync(void)
504504
START_TIMER;
505505
for (ops = 0; alarm_triggered == false; ops++)
506506
{
507-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
507+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
508508
die("could not open output file");
509509
if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
510510
die("write failed");
511511
close(tmpfile);
512512
/* reopen file */
513-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
513+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
514514
die("could not open output file");
515515
if (fsync(tmpfile) != 0)
516516
die("fsync failed");
@@ -535,7 +535,7 @@ test_non_sync(void)
535535
START_TIMER;
536536
for (ops = 0; alarm_triggered == false; ops++)
537537
{
538-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
538+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
539539
die("could not open output file");
540540
if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
541541
die("write failed");

0 commit comments

Comments
 (0)