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

On Fri, Sep 30, 2022 at 6:46 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> +1 for just doing it always, with a one-liner comment like
> "pg_pwritev*() might move the file position". No reason to spam the
> source tree with more explanations of the exact reason.

+1 for resetting the file position in a platform-independent manner.
But, a description there won't hurt IMO and it saves time for the
hackers who spend time there and think why it's that way.

> If someone
> ever comes up with another case where p- and non-p- I/O functions are
> intermixed and it's really worth saving a system call (don't get me
> wrong, we call lseek() an obscene amount elsewhere and I'd like to fix
> that, but this case isn't hot?) then I like your idea of a macro to
> tell you whether you need to.

I don't think we go that route as the code isn't a hot path and an
extra system call wouldn't hurt performance much, a comment there
should work.

> Earlier I wondered why we'd want to include "pg_pwritev" in the name
> of this zero-filling function (pwritev being an internal
> implementation detail), but now it seems like maybe a good idea
> because it highlights the file position portability problem by being a
> member of that family of similarly-named functions.

Hm.

On Thu, Sep 29, 2022 at 10:57 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
>
> + iov[0].iov_base = zbuffer.data;
>
> This seems superfluous, but I don't think it's hurting anything.

Yes, I removed it. Adding a comment, something like [1], would make it
more verbose, hence I've not added.

I'm attaching the v6 patch set, please review it further.

[1]
/*
* Use the first vector buffer to write the remaining size. Note that
* zero buffer was already pointed to it above, hence just specifying
* the size is enough here.
*/

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