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

From: "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Masahiko Sawada' <sawada(dot)mshk(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "dilipbalaut(at)gmail(dot)com" <dilipbalaut(at)gmail(dot)com>, "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: test_decoding assertion failure for the loss of top-sub transaction relationship
Date: 2022-10-12 05:56:02
Message-ID: TYAPR01MB5866B30A1439043B1FC3F21EF5229@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
HEAD-v3-0001-Fix-assertion-failure-during-logical-decoding.patch application/octet-stream 5.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2022-10-12 05:57:16 Re: introduce optimized linear search functions that return index of matching element
Previous Message Michael Paquier 2022-10-12 05:55:50 Re: Temporary file access API