Re: pgsql: Skip empty transaction stream in test_decoding.

On Mon, Nov 9, 2020 at 10:13 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Nov 9, 2020 at 8:40 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Nov 9, 2020 at 7:11 AM Michael Paquier <michael@paquier.xyz> wrote:
> > >
> > > /home/pgbfarm/buildroot/HEAD/pgsql.build/contrib/test_decoding/output_iso/results/concurrent_stream.out
> > > 2020-11-08 12:31:10 -0700
> > > @@ -13,7 +13,6 @@
> > >  opening a streamed block for transaction
> > >  streaming change for transaction
> > >  closing a streamed block for transaction
> > > -committing streamed transaction
> > >  ?column?
> > >
> > > This smells like a race condition.
> > >
> >
> > It is possible. I see that autovacuum has triggered (as per below
> > logs) during this test which might have some impact on the output but
> > I am not sure at this stage. I'll look into this.
> >
>
> I am able to reproduce this issue locally. This is an issue in
> test_decoding. The problem is that in a streaming transaction just
> before the last stream (which has no changes but a commit message)
> another concurrent empty transaction occurred (which in this case is
> caused by autovacuum). Now, in test_dcoding plugin, we track whether
> there is any change occured at the LogicalDecodingContext level
> (xact_wrote_changes) due to which the empty transaction in above
> context can toggle the flag and commit of streaming transaction will
> get confused. The first idea that occured to me to fix this issue is
> to keep track of whether we have decoded any changes at transaction
> level (ReorderBufferTxn) instead of keeping at LogicalDecodingContext
> level as streaming transactions can be interleaved with other
> transactions which was not the case previously. I'll think more on
> this and start a thread on pgsql-hackers.

The analysis seems correct to me,  basically with streaming the actual
streaming messages and the commit can be interleaved so ideally, we
should maintain xact_wrote_changes, within the transaction's context
i.e. ReorderBufferTxn.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com