Re: patch to allow disable of WAL recycling

On Fri, Mar 8, 2019 at 10:13 AM Jerry Jelinek <jerry.jelinek@joyent.com> wrote:
>  I have attached a new version of the patch that implements the changes we've discussed over the past couple of days. Let me know if there are any comments or suggestions.

+        fail = lseek(fd, wal_segment_size - 1, SEEK_SET) < (off_t) 0 ||
+            (int) write(fd, zbuffer.data, 1) != (int) 1;

BTW we now have pg_pwrite() to do this in one syscall.

+        Disabling this option prevents zero-filling new WAL files.
+        This parameter should only be set to <literal>off</literal>
when the WAL
+        resides on a <firstterm>Copy-On-Write</firstterm>
(<acronym>COW</acronym>)
+        filesystem.

Hmm.  The comments in the source give the actual motivation for this
preallocation logic... I wonder why we don't come out and say the same
thing in the documentation, instead of this vague language about COW
filesystems.

Here's a suggestion: "Zero-filling new segment files ensures that it
is safe to use wal_sync_method = fdatasync or wal_sync_method =
open_datasync on filesystems that synchronize file meta-data and data
separately.  It is not necessary on some filesystems such as ZFS."

My understanding is that it's not really the COW-ness that makes it
not necessary, it's the fact that fdatasync() doesn't do anything
different from fsync() on ZFS and there is no situation where
fdatasync() succeeds, you lose power, you come back up and find that
the file size is wrong or a hole in the middle of the file has come
back from the dead, and you lost the data.  The whole concept of "data
sync" implies that file meta-data and file contents are cached and
synchronized separately and you can deliberately ask for weaker
coherency to cut down on IOs; *that's* the thing that ZFS doesn't
have, and couldn't benefit from because it's just going to write stuff
in its tidy sequential log in the form of all-or-nothing transactions
anyway.  I don't know if that's also true for eg BTRFS or any other
COW filesystem that might be out there, but I don't know why you'd
want to mention COW instead of wal_sync_mode as the motivation when
the source code comment know better.

+        Disabling this option prevents WAL file recycling.
+        This parameter should only be set to <literal>off</literal>
when the WAL
+        resides on a COW filesystem.

Would it be better to say what it's for, rather than when to set it?
To make clear that it's a performance setting, not a safety one.
"Setting this option to <literal>off</literal> may increase
performance on copy-on-write filesystems."