Amit Kapila wrote:
> On Fri, Jun 1, 2018 at 8:18 PM, Laurenz Albe <laurenz.a...@cybertec.at> wrote:
> > Amit Kapila wrote:
> > > On Fri, Jun 1, 2018 at 3:13 PM, Laurenz Albe <laurenz.a...@cybertec.at> 
> > > wrote:
> > > > I recently read our documentation about reliability on Windows:
> > > > 
> > > > > On Windows, if wal_sync_method is open_datasync (the default), write 
> > > > > caching can
> > > > > be disabled by unchecking
> > > > > My Computer\Open\disk 
> > > > > drive\Properties\Hardware\Properties\Policies\Enable write caching
> > > > > on the disk. Alternatively, set wal_sync_method to fsync or 
> > > > > fsync_writethrough,
> > > > > which prevent write caching.
> > > > 
> > > > It seems dangerous to me to initialize "wal_sync_method" to a method 
> > > > that is unsafe
> > > > by default.  Admittedly I am not a Windows man, but the fact that this 
> > > > has eluded me
> > > > up to now leads me to believe that other people running PostgreSQL on 
> > > > Windows might
> > > > also have missed that important piece of advice and are consequently 
> > > > running with
> > > > an unsafe setup.
> > > > 
> > > > Wouldn't it be smarter to set a different default value on Windows, 
> > > > like we do on
> > > > Linux (for other reasons)?
> > > > 
> > > 
> > > One thing to note is that it seems that in code we use 
> > > FILE_FLAG_WRITE_THROUGH for
> > > open_datasync which according to MSDN [1] will bypass any intermediate 
> > > cache .
> > > See pgwin32_open.  Have you experimented to set any other option as we 
> > > have a comment
> > > in code which say Win32 only has O_DSYNC?
> > > 
> > > 
> > > [1] - 
> > > https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx
> > 
> > After studying the code I feel somewhat safer; it looks like the code is ok.
> > I have no Windows at hand, so I cannot test right now.
> > 
> > What happened is that I ran "pg_test_fsync" at a customer site on Windows, 
> > and
> > it returned ridiculously high rates got open_datasync.
> > 
> > So I think that the following should be fixed:
> > 
> > - Change pg_test_fsync to actually use FILE_FLAG_WRITE_THROUGH.
> 
> It sounds sensible to me to make a Windows specific change in pg_test_fsync 
> for open_datasync method.
> That will make pg_test_fsync behave similar to server.

The attached patch makes pg_test_fsync use pgwin32_open on Windows, which is 
what
we use elsewhere.  That should fix the problem.
Ist there a better way to do this?  The problem is that "c.h" is only included
at the very end of "postgres-fe.h", which makes front-end code use "open"
rather than "pgwin32_open" on Windows.

Having read it again, I think that the documentation is fine as it is:
After all, this is just advice what you can do if you are running on unsafe 
hardware,
which doesn't flush to disk like it should.

Yours,
Laurenz Albe
From 6ab651c48df66ec93b8d6730bebeaf60e2bb865b Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Tue, 5 Jun 2018 16:05:10 +0200
Subject: [PATCH] Make pg_test_fsync use pgwin32_open on Windows

---
 src/bin/pg_test_fsync/pg_test_fsync.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index e6f7ef8557..a0139a1302 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -3,6 +3,8 @@
  *		tests all supported fsync() methods
  */
 
+/* we have to include c.h first so that pgwin32_open is used on Windows */
+#include "c.h"
 #include "postgres_fe.h"
 
 #include <sys/stat.h>
-- 
2.14.3

Reply via email to