Re: Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes?
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>,
Michael Paquier <michael@paquier.xyz>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-27T19:01:26Z
Lists: pgsql-hackers
On Wed, Sep 28, 2022 at 1:03 AM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:> > On Tue, Sep 27, 2022 at 3:01 PM Thomas Munro <thomas.munro@gmail.com> wrote: > > Something like the attached. > > Isn't it also better to add notes in win32pread.c and win32pwrite.c > about the callers doing lseek(SEEK_SET) if they wish to and Windows > implementations changing file position? We can also add a TODO item > about replacing pg_ versions with pread and friends someday when > Windows fixes the issue? Having it in the commit and include/port.h is > good, but the comments in win32pread.c and win32pwrite.c make life > easier IMO. Otherwise, the patch LGTM. Thanks, will do. FWIW I doubt the OS itself will change released behaviour like that, but I did complain about the straight-up misleading documentation and they agreed and fixed it[1]. After some looking around, the only way I could find to avoid this behaviour is to switch to async handles, which do behave as documented in this respect, but then you can't use plain read/write at all unless you write replacement wrappers for those too, and AFAICT that adds more system calls (start write, wait for write to finish) and complexity/weirdness without any real payoff so it seems like the wrong direction, at least without more research that I'm not pursuing currently. (FWIW in AIO porting experiments a while back we used async handles to get IOs running concurrently with CPU work and possibly other IOs so it was actually worth doing, but that was only for smgr and the main wal writing code where there's no intermixed plain read/write calls as you have here, so the problem doesn't even come up.) [1] https://github.com/MicrosoftDocs/sdk-api/pull/1309
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