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

On Tue, Sep 27, 2022 at 3:08 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> I don't think so, that's an extra kernel call. I think I'll just have
> to revert part of my recent change that removed the pg_ prefix from
> those function names in our code, and restore the comment that warns
> you about the portability hazard (I thought it went away with HP-UX
> 10, where we were literally calling lseek() before every write()).
> The majority of users of these functions don't intermix them with
> calls to read()/write(), so they don't care about the file position,
> so I think it's just something we'll have to continue to be mindful of
> in the places that do.

Yes, all of the existing pwrite() callers don't care about the file
position, but the new callers such as the actual idea and patch
proposed here in this thread cares.

Is this the commit cf112c122060568aa06efe4e6e6fb9b2dd4f1090 part of
which [1] you're planning to revert? If so, will we have the
pg_{pwrite, pread} back? IIUC, we don't have lseek(SEEK_SET) in
pg_{pwrite, pread} right? It is the callers responsibility to set the
file position correctly if they wish to, isn't it? Oftentimes,
developers miss the notes in the function comments and use these
functions expecting them to not change file position which works well
on non-Windows platforms but fails on Windows.

This makes me think that we can have pwrite(), pread() introduced by
cf112c122060568aa06efe4e6e6fb9b2dd4f1090 as-is and re-introduce
pg_{pwrite, pread} with pwrite()/pread()+lseek(SEEK_SET) in
win32pwrite.c and win32pread.c. These functions reduce the caller's
efforts and reduce the duplicate code. If okay, I'm happy to lend my
hands on this patch.

Thoughts?

> Unless, that is, I can find a way to stop it from doing that... I've
> added this to my Windows to-do list. I am going to have a round of
> Windows hacking quite soon.

Thanks! I think it's time for me to start a new thread just for this
to get more attention and opinion from other hackers and not to
sidetrack the original idea and patch proposed in this thread.

[1] https://github.com/postgres/postgres/blob/REL_14_STABLE/src/port/pwrite.c#L11

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com