On Tue, Aug 6, 2024 at 5:36 AM Melih Mutlu <m.melihmutlu@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.
Fixed.
- 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.
Removed the new comment. Only keeping the updated version of the /* Dump the set... */ comment.
- 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?
You're right. I should have thought about the partial write case. I now fixed it by looping and trying to write until compute_remaining_iovec() returns 0.
