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

On Fri, Nov 25, 2022 at 5:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Nov 25, 2022 at 1:35 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > During DecodeCommit() for skipping a transaction we use ReadRecPtr to
> > check whether to skip this transaction or not.  Whereas in
> > ReorderBufferCanStartStreaming() we use EndRecPtr to check whether to
> > stream or not. Generally it will not create a problem but if the
> > commit record itself is adding some changes to the transaction(e.g.
> > snapshot) and if the "start_decoding_at" is in between ReadRecPtr and
> > EndRecPtr then streaming will decide to stream the transaction where
> > as DecodeCommit will decide to skip it.  And for handling this case in
> > ReorderBufferForget() we call stream_abort().
> >
>
> The other cases are probably where we don't have FilterByOrigin or
> dbid check, for example, XLOG_HEAP2_NEW_CID/XLOG_XACT_INVALIDATIONS.
> We anyway actually don't send anything for such cases except empty
> start/stop messages. Can we add some flag to txn which says that there
> is at least one change like DML that we want to stream?
>

We can probably think of using txn_flags for this purpose.

> Then we can
> use that flag to decide whether to stream or not.
>

The other possibility here is to use ReorderBufferTXN's base_snapshot.
Normally, we don't stream unless the base_snapshot is set. However,
there are a few challenges (a) the base_snapshot is set in
SnapBuildProcessChange which is called before DecodeInsert, and
similar APIs. So, now even if we filter out insert due origin or
db_id, the base_snapshot will be set. (b) we currently set it to
execute invalidations even when there are no changes to send to
downstream.

For (a), I guess we can split SnapBuildProcessChange, such that the
part where we set the base snapshot will be done after DecodeInsert
and similar APIs decide to queue the change. I am less sure about
point (b), ideally, if we don't need a snapshot to execute
invalidations, I think we can avoid setting base_snapshot even in that
case. Then we can use it as a check whether we want to stream
anything.

I feel this approach required a lot more changes and bit riskier as
compared to having a flag in txn_flags.

-- 
With Regards,
Amit Kapila.