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

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Nathan Bossart <nathandbossart@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-16T07:58:23Z
Lists: pgsql-hackers
On Wed, Feb 15, 2023 at 01:00:00PM +0530, Bharath Rupireddy wrote:
> The v3 patch reduces initialization of iovec array elements which is a
> clear win when pg_pwrite_zeros is called for sizes less than BLCKSZ
> many times (I assume this is what is needed for the relation extension
> lock improvements feature). However, it increases the number of iovec
> initialization with zerobuf for the cases when pg_pwrite_zeros is
> called for sizes far greater than BLCKSZ (for instance, WAL file
> initialization).

It seems to me that v3 would do extra initializations only if
pg_pwritev_with_retry() does *not* retry its writes, but that's not
the case as it retries on a partial write as per its name.  The number
of iov buffers is stricly capped by remaining_size.  FWIW, I find v3
proposed more elegant.

> FWIW, I attached v4 patch, a simplified version of the v2  - it
> initializes all the iovec array elements if the total blocks to be
> written crosses lengthof(iovec array), otherwise it initializes only
> the needed blocks.

+   static size_t   zbuf_sz = BLCKSZ;
In v4, what's the advantage of marking that as static?  It could
actually be dangerous if this is carelessly updated.  Well, that's not
the case, still..
--
Michael

Commits

  1. Revise pg_pwrite_zeros()

  2. Use pg_pwrite_zeros() in walmethods.c

  3. Introduce pg_pwrite_zeros() in fileutils.c

  4. Move pg_pwritev_with_retry() to src/common/file_utils.c

  5. Restore pg_pread and friends.

  6. Remove dead pread and pwrite replacement code.