On Sun, Jan 10, 2021 at 9:21 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > I left the fsync-after-closing and non-sync'd tests using write(), > because they weren't using lseek(). The latter case is arguably a bit > odd because it's not overwriting pre-allocated blocks, unlike the > earlier tests.
On closer inspection, the weird thing about that final test is that it's opening and closing the file every time. That doesn't seem to make any sense. Perhaps it's a copy and paste error from the previous test? In v2 I changed it to pg_pwrite(), and moved the open and close calls out of the loop.
From 1e6bf5e539d22e12cff133fac6ad2e9ecd24ff3e Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Sat, 9 Jan 2021 23:37:18 +1300 Subject: [PATCH v2] Use pg_pwrite() in pg_test_fsync. For consistency with the PostgreSQL behavior we're tring to simulate, use pwrite() instead of lseek() + write(). Also fix the final "non-sync" test, which was opening and closing the file for every write. Discussion: https://postgr.es/m/CA%2BhUKGJjjid2BJsvjMALBTduo1ogdx2SPYaTQL3wAy8y2hc4nw%40mail.gmail.com --- src/bin/pg_test_fsync/pg_test_fsync.c | 51 +++++++++++++++------------ 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c index 3eddd983c6..29ee7c7d6f 100644 --- a/src/bin/pg_test_fsync/pg_test_fsync.c +++ b/src/bin/pg_test_fsync/pg_test_fsync.c @@ -290,10 +290,11 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pg_pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); - if (lseek(tmpfile, 0, SEEK_SET) == -1) - die("seek failed"); } STOP_TIMER; close(tmpfile); @@ -315,11 +316,12 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pg_pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); fdatasync(tmpfile); - if (lseek(tmpfile, 0, SEEK_SET) == -1) - die("seek failed"); } STOP_TIMER; close(tmpfile); @@ -339,12 +341,13 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pg_pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); if (fsync(tmpfile) != 0) die("fsync failed"); - if (lseek(tmpfile, 0, SEEK_SET) == -1) - die("seek failed"); } STOP_TIMER; close(tmpfile); @@ -362,12 +365,13 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pg_pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) die("write failed"); if (pg_fsync_writethrough(tmpfile) != 0) die("fsync failed"); - if (lseek(tmpfile, 0, SEEK_SET) == -1) - die("seek failed"); } STOP_TIMER; close(tmpfile); @@ -393,8 +397,10 @@ test_sync(int writes_per_op) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < writes_per_op; writes++) - if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) - + if (pg_pwrite(tmpfile, + buf, + XLOG_BLCKSZ, + writes * XLOG_BLCKSZ) != XLOG_BLCKSZ) /* * This can generate write failures if the filesystem has * a large block size, e.g. 4k, and there is no support @@ -402,8 +408,6 @@ test_sync(int writes_per_op) * size, e.g. XFS. */ die("write failed"); - if (lseek(tmpfile, 0, SEEK_SET) == -1) - die("seek failed"); } STOP_TIMER; close(tmpfile); @@ -457,11 +461,12 @@ test_open_sync(const char *msg, int writes_size) for (ops = 0; alarm_triggered == false; ops++) { for (writes = 0; writes < 16 / writes_size; writes++) - if (write(tmpfile, buf, writes_size * 1024) != + if (pg_pwrite(tmpfile, + buf, + writes_size * 1024, + writes * writes_size * 1024) != writes_size * 1024) die("write failed"); - if (lseek(tmpfile, 0, SEEK_SET) == -1) - die("seek failed"); } STOP_TIMER; close(tmpfile); @@ -553,16 +558,16 @@ test_non_sync(void) printf(LABEL_FORMAT, "write"); fflush(stdout); + 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++) { - if ((tmpfile = open(filename, O_RDWR | PG_BINARY, 0)) == -1) - die("could not open output file"); - if (write(tmpfile, buf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (pg_pwrite(tmpfile, buf, XLOG_BLCKSZ, 0) != XLOG_BLCKSZ) die("write failed"); - close(tmpfile); } STOP_TIMER; + close(tmpfile); } static void -- 2.20.1
