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

On Tue, Sep 6, 2022 at 1:17 PM kuroda(dot)hayato(at)fujitsu(dot)com
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear hackers,
>
> > I agreed both that DEBUG2 messages are still useful but we should not
> > change the condition for output. So I prefer the idea suggested by Amit.
>
> PSA newer patch, which contains the fix and test.
>
> > > I have not verified but I think we need to backpatch this till 14
> > > because prior to that in DecodeCommit, we use to set the top-level txn
> > > as having catalog changes based on the if there are invalidation
> > > messages in the commit record. So, in the current scenario shared by
> > > Osumi-San, before SnapBuildCommitTxn(), the top-level txn will be
> > > marked as having catalog changes.
> >
> > I and Osumi-san are now investigating that, so please wait further reports and
> > patches.
>
> We investigated it about older versions, and in some versions *another stack-trace* has been found.
>
>
> About PG10-13, indeed, the failure was not occurred.
> In these versions transactions are regarded as
> that have catalog changes when the commit record has XACT_XINFO_HAS_INVALS flag.
> This flag will be set if the transaction has invalidation messages.
>
> When sub transaction changes system catalogs and user commits,
> all invalidation messages allocated in sub transaction will be transferred to top transaction.
> Therefore both transactions will be marked as containing catalog changes.
>
>
> About PG14 and 15, however, another stack-trace has been found.
> While executing the same workload, we got followings at the same SQL statement;
>

Did you get this new assertion failure after you applied the patch for
the first failure? Because otherwise, how can you reach it with the
same test case?

About patch:
else if (sub_needs_timetravel)
{
- /* track toplevel txn as well, subxact alone isn't meaningful */
+ elog(DEBUG2, "forced transaction %u to do timetravel due to one of
its subtransaction",
+ xid);
+ needs_timetravel = true;
SnapBuildAddCommittedTxn(builder, xid);

Why did you remove the above comment? I think it still makes sense to retain it.

--
With Regards,
Amit Kapila.