On Fri, Nov 25, 2022 at 5:38 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Nov 25, 2022 at 1:35 PM Dilip Kumar <dilipbal...@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.