On Sun, Nov 27, 2022 1:33 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Sat, Nov 26, 2022 at 12:15 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > > > 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. > > In the attached patch I have used txn_flags to identify whether it has > any streamable change or not and the transaction will not be selected > for streaming unless it has at least one streamable change. >
Thanks for your patch. I saw that the patch added a check when selecting largest transaction, but in addition to ReorderBufferCheckMemoryLimit(), the transaction can also be streamed in ReorderBufferProcessPartialChange(). Should we add the check in this function, too? diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 9a58c4bfb9..108737b02f 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -768,7 +768,8 @@ ReorderBufferProcessPartialChange(ReorderBuffer *rb, ReorderBufferTXN *txn, */ if (ReorderBufferCanStartStreaming(rb) && !(rbtxn_has_partial_change(toptxn)) && - rbtxn_is_serialized(txn)) + rbtxn_is_serialized(txn) && + rbtxn_has_streamable_change(txn)) ReorderBufferStreamTXN(rb, toptxn); } Regards, Shi yu