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

At Mon, 13 Feb 2023 10:15:03 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in 
> On Sun, Feb 12, 2023 at 11:01 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > On 2023-02-12 19:59:00 +0530, Bharath Rupireddy wrote:
> > >       /* Prepare to write out a lot of copies of our zero buffer at once. */
> > >       for (i = 0; i < lengthof(iov); ++i)
> > >       {
> > > -             iov[i].iov_base = zbuffer.data;
> > > +             iov[i].iov_base = (void *) (unconstify(PGAlignedBlock *, &zbuffer)->data);
> > >               iov[i].iov_len = zbuffer_sz;
> > >       }
> >
> > Another thing: I think we should either avoid iterating over all the IOVs if
> > we don't need them, or, even better, initialize the array as a constant, once.

FWIW, I tried to use the "{[start .. end] = {}}" trick (GNU extension?
[*1]) for constant array initialization, but individual members don't
accept assigning a const value, thus I did deconstify as the follows.

>	static const struct iovec	iov[PG_IOV_MAX] =
>		{[0 ... PG_IOV_MAX - 1] =
>		 {
>			 .iov_base = (void *)&zbuffer.data,
>			 .iov_len = BLCKSZ
>		 }
>		};

I didn't checked the actual mapping, but if I tried an assignment
"iov[0].iov_base = NULL", it failed as "assignment of member
‘iov_base’ in read-only object", so is it successfully placed in a
read-only segment?

Later code assigns iov[0].iov_len thus we need to provide a separate
iov non-const variable, or can we use pwrite instead there?  (I didn't
find pg_pwrite_with_retry(), though)

> How about like the attached patch? It makes the iovec static variable
> and points the zero buffer only once/for the first time to iovec. This
> avoids for-loop on every call.

As the patch itself, it seems forgetting to reset iov[0].iov_len after
writing a partial block.


retards.


*1: https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Designated-Inits.html

-- 
Kyotaro Horiguchi
NTT Open Source Software Center