Re: [HACKERS] logical decoding of two-phase transactions - Mailing list pgsql-hackers

From Peter Smith
Subject Re: [HACKERS] logical decoding of two-phase transactions
Date
Msg-id CAHut+Ptqd-fpwpUwE8tgx1fV_qrP7Q8Xyz43y5QNrTxx+F7uQA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] logical decoding of two-phase transactions  (vignesh C <vignesh21@gmail.com>)
List pgsql-hackers
On Mon, May 10, 2021 at 1:31 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, Apr 29, 2021 at 2:23 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Please find attached the latest patch set v74*
> >
> > Differences from v73* are:
> >
> > * Rebased to HEAD @ 2 days ago.
> >
> > * v74 addresses most of the feedback comments from Vignesh posts [1][2][3].
> >
>
> Thanks for the updated patch.
> Few comments:
> 1) I felt skey[2] should be skey as we are just using one key here.
>
> +       ScanKeyData skey[2];
> +       SysScanDesc scan;
> +       bool            has_subrels;
> +
> +       rel = table_open(SubscriptionRelRelationId, AccessShareLock);
> +
> +       ScanKeyInit(&skey[nkeys++],
> +                               Anum_pg_subscription_rel_srsubid,
> +                               BTEqualStrategyNumber, F_OIDEQ,
> +                               ObjectIdGetDatum(subid));
> +
> +       scan = systable_beginscan(rel, InvalidOid, false,
> +                                                         NULL, nkeys, skey);
> +
>

Fixed in v75.

> 2) I felt we can change lsn data type from Int64 to XLogRecPtr
> +<varlistentry>
> +<term>Int64</term>
> +<listitem><para>
> +                The LSN of the prepare.
> +</para></listitem>
> +</varlistentry>
> +
> +<varlistentry>
> +<term>Int64</term>
> +<listitem><para>
> +                The end LSN of the transaction.
> +</para></listitem>
> +</varlistentry>

Deferred.

>
> 3) I felt we can change lsn data type from Int32 to TransactionId
> +<varlistentry>
> +<term>Int32</term>
> +<listitem><para>
> +                Xid of the subtransaction (will be same as xid of the
> transaction for top-level
> +                transactions).
> +</para></listitem>
> +</varlistentry>

Deferred.

>
> 4) Should we change this to "The end LSN of the prepared transaction"
> just to avoid any confusion of it meaning commit/rollback.
> +<varlistentry>
> +<term>Int64</term>
> +<listitem><para>
> +                The end LSN of the transaction.
> +</para></listitem>
> +</varlistentry>
>

Modified in v75 for message types 'b', 'P', 'K', 'r', 'p'.

> Similar problems related to comments 2 and 3 are being discussed at
> [1], we can change it accordingly based on the conclusion in the other
> thread.
> [1] -
https://www.postgresql.org/message-id/flat/CAHut%2BPs2JsSd_OpBR9kXt1Rt4bwyXAjh875gUpFw6T210ttO7Q%40mail.gmail.com#cf2a85d0623dcadfbb1204a196681313

Yes, I will defer addressing those feedback comments 2 and 3 pending
the outcome of your other patch of the above thread.

----------
Kind Regards,
Peter Smith.
Fujitsu Australia



pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: [HACKERS] logical decoding of two-phase transactions
Next
From: torikoshia
Date:
Subject: Re: RFC: Logging plan of the running query