On Mon, Nov 9, 2020 at 1:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Nov 9, 2020 at 11:31 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Nov 9, 2020 at 11:21 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Mon, Nov 9, 2020 at 11:04 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Mon, Nov 9, 2020 at 11:00 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > > Michael reported a BF failure [1] related to one of the logical
> > > > > streaming test case and I've analyzed the issue. As responded on
> > > > > pgsql-committers [2], the issue here is that the streaming
> > > > > transactions can be interleaved and because we are maintaining whether
> > > > > xact_wrote_changes at the LogicalDecodingContext level, one of later
> > > > > transaction can overwrite the flag for previously streaming
> > > > > transaction. I think it is logical to have this flag at each
> > > > > transaction level (aka in ReorderBufferTxn), however till now it was
> > > > > fine because the changes of each transaction are decoded at one-shot
> > > > > which will be no longer true. We can keep a output_plugin_private data
> > > > > pointer in ReorderBufferTxn which will be used by test_decoding module
> > > > > to keep this and any other such flags in future. We need to set this
> > > > > flag at begin_cb and stream_start_cb APIs and then reset/remove it at
> > > > > stream_commit_cb, stream_abort_cb and stream_stop_cb APIs.
> > >
> > > So IIUC, we need to keep 'output_plugin_private' in
> > > LogicalDecodingContext as well as in ReorderBufferTxn, So the
> > > output_plugin_private in the ReorderBufferTxn will currently just keep
> > > one flag xact_wrote_changes and the remaining things will still be
> > > maintained in output_plugin_private of the LogicalDecodingContext. Is
> > > my understanding correct?
> > >
> >
> > Yes. But keep it as void * so that we can add more things later if required.
>
> Yeah, that makes sense to me.
I have made some POC changes and analyzed this further, I think that
for the streaming transaction we need 2 flags
1) xact_wrote_changes 2) stream_wrote_changes
So basically, if the stream didn't make any changes we can skip the
stream start and stream stop message for the empty stream, but if any
of the streams has made any change then we need to emit the
transaction commit message. But if we want to avoid tracking the
changes per stream then maybe once we set the xact_wrote_changes to
true once for the txn then we better emit the message for all the
stream without tracking whether the stream is empty or not. What is
your thought on this?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
