Re: test_decoding assertion failure for the loss of top-sub transaction relationship

At Fri, 2 Sep 2022 10:59:56 +0530, Dilip Kumar <dilipbalaut@gmail.com> wrote in 
> On Fri, Sep 2, 2022 at 6:38 AM kuroda.hayato@fujitsu.com
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Hi Hackers,
> >
> > > Therefore, this leads to the failure for the assert that can check
> > > the consistency that when one sub transaction modifies the catalog,
> > > its top transaction should be marked so as well.
> > >
> > > I feel we need to remember the relationship between top transaction and sub
> > > transaction
> > > in the serialized snapshot even before changing catalog at decoding
> > > RUNNING_XACT,
> > > so that we can keep track of the association after the restart. What do you think ?
> >
> > PSA patch that fixes the failure.
> > This adds pairs of sub-top transactions to the SnapBuild, and it will be serialized and restored.
> > The pair will be checked when we mark the ReorderBufferTXN as RBTXN_HAS_CATALOG_CHANGES.
> 
> It seems that SnapBuildCommitTxn() is already taking care of adding
> the top transaction to the committed transaction if any subtransaction
> has the catalog changes, it has just missed setting the flag so I
> think just setting the flag like this should be sufficient no?

Oops! That's right.

> diff --git a/src/backend/replication/logical/snapbuild.c
> b/src/backend/replication/logical/snapbuild.c
> index 1ff2c12..ee3f695 100644
> --- a/src/backend/replication/logical/snapbuild.c
> +++ b/src/backend/replication/logical/snapbuild.c
> @@ -1086,6 +1086,7 @@ SnapBuildCommitTxn(SnapBuild *builder,
> XLogRecPtr lsn, TransactionId xid,
>         else if (sub_needs_timetravel)
>         {
>                 /* track toplevel txn as well, subxact alone isn't meaningful */
> +               needs_timetravel = true;
>                 SnapBuildAddCommittedTxn(builder, xid);
>         }
>         else if (needs_timetravel)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center