On Wed, Sep 28, 2022 at 1:03 AM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote:> > On Tue, Sep 27, 2022 at 3:01 PM Thomas Munro <thomas.mu...@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