Re: Vectored IO in XLogWrite()

Hi Robert,

Thanks for reviewing.

Robert Haas <robertmhaas(at)gmail(dot)com>, 6 Ağu 2024 Sal, 20:43 tarihinde şunu
yazdı:

> On Tue, Aug 6, 2024 at 5:36 AM Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)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.

Thanks,
--
Melih Mutlu
Microsoft