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
unnecessaryto 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
contiguousin 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
