Re: Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes?
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: bharath.rupireddyforpostgres@gmail.com, michael@paquier.xyz, alvherre@alvh.no-ip.org, nathandbossart@gmail.com, thomas.munro@gmail.com, pgsql-hackers@lists.postgresql.org
Date: 2023-02-13T17:39:47Z
Lists: pgsql-hackers
On 2023-02-13 18:33:34 +0900, Kyotaro Horiguchi wrote:
> 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)
Given that we need to do that, and given that we already need to loop to
handle writes that are longer than PG_IOV_MAX * BLCKSZ, it's probably not
worth avoiding iov initialization.
But I think it's worth limiting the initialization to blocks.
I'd also try to combine the first pg_writev_* with the second one.
Greetings,
Andres Freund
Commits
-
Revise pg_pwrite_zeros()
- ce340e530d1f 16.0 landed
-
Use pg_pwrite_zeros() in walmethods.c
- 28cc2976a9cf 16.0 landed
-
Introduce pg_pwrite_zeros() in fileutils.c
- 3bdbdf5d06f2 16.0 landed
-
Move pg_pwritev_with_retry() to src/common/file_utils.c
- 4ab8c81bd90a 16.0 landed
-
Restore pg_pread and friends.
- b6d8a60aba32 16.0 landed
-
Remove dead pread and pwrite replacement code.
- cf112c122060 16.0 cited