Re: Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes?

On Thu, Sep 29, 2022 at 11:32:32AM +0530, Bharath Rupireddy wrote:
> On Sat, Sep 24, 2022 at 1:54 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>>
>> + PGAlignedXLogBlock zbuffer;
>> +
>> + memset(zbuffer.data, 0, XLOG_BLCKSZ);
>>
>> This seems excessive for only writing a single byte.
>
> Yes, I removed it now, instead doing pg_pwrite(fd, "\0", 1,
> wal_segment_size - 1).

I don't think removing the use of PGAlignedXLogBlock here introduces any
sort of alignment risk, so this should be alright.

+#ifdef WIN32
+ /*
+ * WriteFile() on Windows changes the current file position, hence we
+ * need an explicit lseek() here. See pg_pwrite() implementation in
+ * win32pwrite.c for more details.
+ */

Should we really surround this with a WIN32 check, or should we just
unconditionally lseek() here? I understand that this helps avoid an extra
system call on many platforms, but in theory another platform introduced in
the future could have the same problem, and this seems like something that
could easily be missed. Presumably we could do something fancier to
indicate pg_pwrite()'s behavior in this regard, but I don't know if that
sort of complexity is really worth it in order to save an lseek().

+ iov[0].iov_base = zbuffer.data;

This seems superfluous, but I don't think it's hurting anything.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com