On Thu, May 24, 2018 at 10:14:01AM +0900, Kyotaro HORIGUCHI wrote: > At Wed, 23 May 2018 15:56:22 +0900, Masahiko Sawada <sawada.m...@gmail.com> > wrote in <CAD21AoA+5Tz0Z8zHOmD=sU5F=cygoejhs7wvbbdl07fh9ay...@mail.gmail.com> >> Another possible way might be to make XLogFindNextRecord valid in >> backend code and move startlsn to the first valid record with an lsn >> >= startlsn by using that function. Please find attached patch. > > The another reason for the code is the fact that confirmed_lsn is > storing EndRecPtr after the last XLogReadRecord call. That is, > from the definition, confirmed_lsn must be on the start of a > record or page boundary and error out if not. For that reason, > calling XLogFindNextRecord would not be the right thing to do > here. We should just skip a header if we are on a boundary but it > already done in XLogReadRecord.
Maybe I am being too naive, but wouldn't it be just enough to update the
confirmed flush LSN to ctx->reader->ReadRecPtr? This way, the slot
advances up to the beginning of the last record where user wants to
advance, and not the beginning of the next record:
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -395,7 +395,7 @@ pg_logical_replication_slot_advance(XLogRecPtr startlsn,
XLogRecPtr moveto)
if (ctx->reader->EndRecPtr != InvalidXLogRecPtr)
{
- LogicalConfirmReceivedLocation(moveto);
+ LogicalConfirmReceivedLocation(ctx->reader->ReadRecPtr);
/*
* If only the confirmed_flush_lsn has changed the slot won't get
I agree with the point made above to not touch manually the XLogReader
context out of xlogreader.c.
--
Michael
signature.asc
Description: PGP signature
