On Tue, 10 Jan 2023 at 15:08, Andres Freund <and...@anarazel.de> wrote: > Thanks for letting me now. Updated version attached.
I'm not too sure I've qualified for giving a meaningful design review here, but I have started looking at the patches and so far only made it as far as 0006. I noted down the following while reading: v2-0001: 1. BufferCheckOneLocalPin needs a header comment v2-0002: 2. The following comment and corresponding code to release the extension lock has been moved now. /* * Release the file-extension lock; it's now OK for someone else to extend * the relation some more. */ I think it's worth detailing out why it's fine to release the extension lock in the new location. You've added detail to the commit message but I think you need to do the same in the comments too. v2-0003 3. FileFallocate() and FileZero() should likely document what they return, i.e zero on success and non-zero on failure. 4. I'm not quite clear on why you've modified FileGetRawDesc() to call FileAccess() twice. v2-0004: 5. Is it worth having two versions of PinLocalBuffer() one to adjust the usage count and one that does not? Couldn't the version that does not adjust the count skip doing pg_atomic_read_u32()? v2-0005 v2-0006 David
