On 8/17/21, 1:24 PM, "alvherre@alvh.no-ip.org" <alvherre@alvh.no-ip.org> wrote:
> The thing I still don't understand about this patch is why we call
> RegisterSegmentBoundaryEntry and NotifySegmentsReadyForArchive in
> XLogInsertRecord.
>
> My model concept of this would have these routines called only in
> XLogFlush / XLogWrite, which are conceptually about transferring data
> from in-memory WAL buffers into the on-disk WAL store (pg_xlog files).
> As I understand, XLogInsertRecord is about copying bytes from the
> high-level operation (heap insert etc) into WAL buffers. So doing the
> register/notify dance in both places should be redundant and
> unnecessary.
The main reason for registering the boundaries in XLogInsertRecord()
is that it has the required information about the WAL record
boundaries. I do not think that XLogWrite() has this information.
If we assumed that write requests always pointed to record boundaries,
we could probably just move the XLogArchiveNotifySeg() calls to the
end of XLogWrite(), which is what my original patch [0] did.
> In the NotifySegmentsReadyForArchive() call at the bottom of XLogWrite,
> we use flushed=InvalidXLogRecPtr. Why is that? Surely we can use
> LogwrtResult.Flush, just like in the other callsite there? (If we're
> covering for somebody advancing FlushRecPtr concurrently, I think we
> add a comment to explain that. But why do we need to do that in the
> first place?)
Good point. I did this in the new version of the patch.
Nathan
[0] https://postgr.es/m/CBDDFA01-6E40-46BB-9F98-9340F4379505%40amazon.com