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

On Fri, Sep 2, 2022 at 6:38 AM kuroda(dot)hayato(at)fujitsu(dot)com
<kuroda(dot)hayato(at)fujitsu(dot)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?

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,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com