Re: Loaded footgun open_datasync on Windows

Amit Kapila wrote:
> On Fri, Jun 1, 2018 at 3:13 PM, Laurenz Albe <laurenz.albe@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.
  Currently it uses O_DSYNC, which is defined in port/win32_port.h as

  /*
   * Supplement to <fcntl.h>.
   * This is the same value as _O_NOINHERIT in the MS header file. This is
   * to ensure that we don't collide with a future definition. It means
   * we cannot use _O_NOINHERIT ourselves.
   */
  #define O_DSYNC 0x0080

- Change the documentation so that it does not claim that open_datasync is unsafe
  unless you change the device settings.

If there is a consensus that this is fine, I can do the legwork.

Yours,
Laurenz Albe