On Tue, Aug 6, 2024 at 5:36 AM Melih Mutlu <m.melihmu...@gmail.com> wrote: > I think that we don't have the "contiguous pages" constraint when writing > anymore as we can do vectored IO. It seems unnecessary to write just because > XLogWrite() is at the end of wal buffers. > Attached patch uses pg_pwritev() instead of pg_pwrite() and tries to write > pages in one call even if they're not contiguous in memory, until it reaches > the page at startidx.
Here are a few notes on this patch: - It's not pgindent-clean. In fact, it doesn't even pass git diff --check. - You added a new comment (/* Reaching the buffer... */) in the middle of a chunk of lines that were already covered by an existing comment (/* Dump the set ... */). This makes it look like the /* Dump the set... */ comment only covers the 3 lines of code that immediately follow it rather than everything in the "if" statement. You could fix this in a variety of ways, but in this case the easiest solution, to me, looks like just skipping the new comment. It seems like the point is pretty self-explanatory. - The patch removes the initialization of "from" but not the variable itself. You still increment the variable you haven't initialized. - I don't think the logic is correct after a partial write. Pre-patch, "from" advances while "nleft" goes down, but post-patch, what gets written is dependent on the contents of "iov" which is initialized outside the loop and never updated. Perhaps compute_remaining_iovec would be useful here? - I assume this is probably a good idea in principle, because fewer system calls are presumably better than more. The impact will probably be very small, though. -- Robert Haas EDB: http://www.enterprisedb.com
