Skip to content

Commit cd113a0

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 9292747 commit cd113a0

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
@@ -225,7 +225,7 @@ test_open(void)
225225
/*
226226
* test if we can open the target file
227227
*/
228-
if ((tmpfile = open(filename, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) == -1)
228+
if ((tmpfile = open(filename, O_RDWR | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR)) == -1)
229229
die("could not open output file");
230230
needs_unlink = 1;
231231
if (write(tmpfile, full_buf, DEFAULT_XLOG_SEG_SIZE) !=
@@ -260,7 +260,7 @@ test_sync(int writes_per_op)
260260
fflush(stdout);
261261

262262
#ifdef OPEN_DATASYNC_FLAG
263-
if ((tmpfile = open(filename, O_RDWR | O_DSYNC | PG_O_DIRECT, 0)) == -1)
263+
if ((tmpfile = open(filename, O_RDWR | O_DSYNC | PG_O_DIRECT | PG_BINARY, 0)) == -1)
264264
{
265265
printf(NA_FORMAT, _("n/a*"));
266266
fs_warning = true;
@@ -290,7 +290,7 @@ test_sync(int writes_per_op)
290290
fflush(stdout);
291291

292292
#ifdef HAVE_FDATASYNC
293-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
293+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
294294
die("could not open output file");
295295
START_TIMER;
296296
for (ops = 0; alarm_triggered == false; ops++)
@@ -314,7 +314,7 @@ test_sync(int writes_per_op)
314314
printf(LABEL_FORMAT, "fsync");
315315
fflush(stdout);
316316

317-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
317+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
318318
die("could not open output file");
319319
START_TIMER;
320320
for (ops = 0; alarm_triggered == false; ops++)
@@ -337,7 +337,7 @@ test_sync(int writes_per_op)
337337
fflush(stdout);
338338

339339
#ifdef HAVE_FSYNC_WRITETHROUGH
340-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
340+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
341341
die("could not open output file");
342342
START_TIMER;
343343
for (ops = 0; alarm_triggered == false; ops++)
@@ -363,7 +363,7 @@ test_sync(int writes_per_op)
363363
fflush(stdout);
364364

365365
#ifdef OPEN_SYNC_FLAG
366-
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT, 0)) == -1)
366+
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT | PG_BINARY, 0)) == -1)
367367
{
368368
printf(NA_FORMAT, _("n/a*"));
369369
fs_warning = true;
@@ -430,7 +430,7 @@ test_open_sync(const char *msg, int writes_size)
430430
fflush(stdout);
431431

432432
#ifdef OPEN_SYNC_FLAG
433-
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT, 0)) == -1)
433+
if ((tmpfile = open(filename, O_RDWR | OPEN_SYNC_FLAG | PG_O_DIRECT | PG_BINARY, 0)) == -1)
434434
printf(NA_FORMAT, _("n/a*"));
435435
else
436436
{
@@ -478,7 +478,7 @@ test_file_descriptor_sync(void)
478478
START_TIMER;
479479
for (ops = 0; alarm_triggered == false; ops++)
480480
{
481-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
481+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
482482
die("could not open output file");
483483
if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
484484
die("write failed");
@@ -490,7 +490,7 @@ test_file_descriptor_sync(void)
490490
* open and close the file again to be consistent with the following
491491
* test
492492
*/
493-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
493+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
494494
die("could not open output file");
495495
close(tmpfile);
496496
}
@@ -506,13 +506,13 @@ test_file_descriptor_sync(void)
506506
START_TIMER;
507507
for (ops = 0; alarm_triggered == false; ops++)
508508
{
509-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
509+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
510510
die("could not open output file");
511511
if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
512512
die("write failed");
513513
close(tmpfile);
514514
/* reopen file */
515-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
515+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
516516
die("could not open output file");
517517
if (fsync(tmpfile) != 0)
518518
die("fsync failed");
@@ -537,7 +537,7 @@ test_non_sync(void)
537537
START_TIMER;
538538
for (ops = 0; alarm_triggered == false; ops++)
539539
{
540-
if ((tmpfile = open(filename, O_RDWR, 0)) == -1)
540+
if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1)
541541
die("could not open output file");
542542
if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
543543
die("write failed");

0 commit comments

Comments
 (0)