On Thu, Apr 10, 2025 at 06:45:36PM -0500, Sami Imseih wrote: > IIUC, this is only an issue for wal syncing
Yes, good catch. I have missed this effect of issue_xlog_fsync(),
which has two callers. The first one in XLogWrite() never happens if
wal_sync_method is open_sync or open_datasync. The second call just
relies on the sync to be discarded internally by issue_xlog_fsync().
> so only the below test with object = 'wal' needs to be tightened
>
> SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
> FROM pg_stat_io
> WHERE context = 'normal' AND object = 'wal' \gset io_sum_wal_normal_after_
My mistake here. Sorry about that.
SELECT current_setting('fsync') = 'off'
+ OR current_setting('wal_sync_method') NOT IN ('fdatasync', 'fsync',
'fsync_writethrough')
The code in xlog.c filters out the syncs for WAL_SYNC_METHOD_OPEN and
WAL_SYNC_METHOD_OPEN_DSYNC, wouldn't it be more consistent to do the
same in the code and the SQL test, using an IN clause with the two
values that block the syncs rather than a NOT IN clause with the three
values that allow the syncs? This translates to the attached, which
is the same as your patch, but this is more consistent with the code.
--
Michael
From c4c8993275e27cc5fde38f4d24ad223beb2debd4 Mon Sep 17 00:00:00 2001
From: Sami Imseih <sims...@amazon.com>
Date: Fri, 11 Apr 2025 13:16:28 +0900
Subject: [PATCH v2] Fix stats.sql test for systems that don't sync wal with
fsync
---
src/test/regress/expected/stats.out | 1 +
src/test/regress/sql/stats.sql | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index cd08a2ca0af1..776f1ad0e534 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1456,6 +1456,7 @@ SELECT :io_sum_wal_normal_after_writes > :io_sum_wal_normal_before_writes;
(1 row)
SELECT current_setting('fsync') = 'off'
+ OR current_setting('wal_sync_method') IN ('open_sync', 'open_datasync')
OR :io_sum_wal_normal_after_fsyncs > :io_sum_wal_normal_before_fsyncs;
?column?
----------
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index c223800fd19c..232ab8db8fa8 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -672,6 +672,7 @@ SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
SELECT current_setting('synchronous_commit') = 'on';
SELECT :io_sum_wal_normal_after_writes > :io_sum_wal_normal_before_writes;
SELECT current_setting('fsync') = 'off'
+ OR current_setting('wal_sync_method') IN ('open_sync', 'open_datasync')
OR :io_sum_wal_normal_after_fsyncs > :io_sum_wal_normal_before_fsyncs;
-- Change the tablespace so that the table is rewritten directly, then SELECT
--
2.49.0
signature.asc
Description: PGP signature
