Hi Robert, Thanks for reviewing.
Robert Haas <robertmh...@gmail.com>, 6 Ağu 2024 Sal, 20:43 tarihinde şunu yazdı: > 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. > 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
v2-0001-Use-pg_pwritev-in-XlogWrite.patch
Description: Binary data
