Re: Avoid streaming the transaction which are skipped (in corner cases)

Hi Dilip,

On Tue, Nov 29, 2022 at 9:38 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> >
> > You are right we need this in ReorderBufferProcessPartialChange() as
> > well.  I will fix this in the next version.
>
> Fixed this in the attached patch.
>

I focused my attention on SnapBuildXactNeedsSkip() usages and I see
they are using different end points of WAL record
1 decode.c        logicalmsg_decode               594
SnapBuildXactNeedsSkip(builder, buf->origptr)))
2 decode.c        DecodeTXNNeedSkip              1250 return
(SnapBuildXactNeedsSkip(ctx->snapshot_builder, buf->origptr) ||
3 reorderbuffer.c AssertTXNLsnOrder               897 if
(SnapBuildXactNeedsSkip(ctx->snapshot_builder,
ctx->reader->EndRecPtr))
4 reorderbuffer.c ReorderBufferCanStartStreaming 3922
!SnapBuildXactNeedsSkip(builder, ctx->reader->EndRecPtr))
5 snapbuild.c     SnapBuildXactNeedsSkip          429
SnapBuildXactNeedsSkip(SnapBuild *builder, XLogRecPtr ptr)

The first two are using origin ptr and the last two are using end ptr.
you have fixed the fourth one. Do we need to fix the third one as
well?

Probably we need to create two wrappers (macros) around
SnapBuildXactNeedsSkip(), one which accepts a XLogRecordBuffer and
other which accepts XLogReaderState. Then use those. That way at least
we have logic unified as to which XLogRecPtr to use.

--
Best Wishes,
Ashutosh Bapat