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

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
To: 'Masahiko Sawada' <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "dilipbalaut@gmail.com" <dilipbalaut@gmail.com>, "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2022-10-12T05:56:02Z
Lists: pgsql-hackers

Attachments

Dear Sawada-san,

Thank you for reviewing HEAD patch! PSA v3 patch.

> +# Test that we can force the top transaction to do timetravel when one of sub
> +# transactions needs that. This is necessary when we restart decoding
> from RUNNING_XACT
> +# without the wal to associate subtransaction to its top transaction.
> 
> I don't think the second sentence is necessary.
> 
> ---
> The last decoding
> +# starts from the first checkpoint and NEW_CID of "s0_truncate"
> doesn't mark the top
> +# transaction as catalog modifying transaction. In this scenario, the
> enforcement sets
> +# needs_timetravel to true even if the top transaction is regarded as
> that it does not
> +# have catalog changes and thus the decoding works without a
> contradition that one
> +# subtransaction needed timetravel while its top transaction didn't.
> 
> I don't understand the last sentence, probably it's a long sentence.
> 
> How about the following description?
> 
> # Test that we can handle the case where only subtransaction is marked
> as containing
> # catalog changes. The last decoding starts from NEW_CID generated by
> "s0_truncate" and
> # marks only the subtransaction as containing catalog changes but we
> don't create the
> # association between top-level transaction and subtransaction yet.
> When decoding the
> # commit record of the top-level transaction, we must force the
> top-level transaction
> # to do timetravel since one of its subtransactions is marked as
> containing catalog changes.

Seems good, I replaced all of comments to yours.

> + elog(DEBUG2, "forced transaction %u to do timetravel due to one of
> its subtransaction",
> + xid);
> + needs_timetravel = true;
> 
> I think "one of its subtransaction" should be "one of its subtransactions".

Fixed. 

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. Fix executing invalidation messages generated by subtransactions during decoding.

  2. Fix assertion failures while processing NEW_CID record in logical decoding.