Horton

Re: Slow catchup of 2PC (twophase) transactions on replica in LR

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Vitaly Davydov <v.davydov@postgrespro.ru>, Ajin Cherian <itsajin@gmail.com>
Date: 2024-07-22T03:15:40Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix random failure in 021_twophase.

  2. Allow altering of two_phase option of a SUBSCRIPTION.

  3. Doc: use true|false rather than on|off for "failover" option

  4. Support an optional asynchronous commit mode, in which we don't flush WAL

On Sat, Jul 20, 2024 at 9:31 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, 18 Jul 2024 at 07:41, Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Peter,
> >
> > Thanks for giving comments! PSA new version.
>
> Couple of suggestions:
> 1) How will user know which all transactions should be rolled back
> since the prepared transaction name will be different in subscriber
> like pg_gid_16398_750, can we mention some info on how user can
> identify these prepared transactions that should be rolled back in the
> subscriber or if this information is already available can we point it
> from here:
> +      When altering <link
> linkend="sql-createsubscription-params-with-two-phase"><literal>two_phase</literal></link>
> +      from <literal>true</literal> to <literal>false</literal>, the backend
> +      process reports and an error if any prepared transactions done by the
> +      logical replication worker (from when <literal>two_phase</literal>
> +      parameter was still <literal>true</literal>) are found. You can resolve
> +      prepared transactions on the publisher node, or manually roll back them
> +      on the subscriber, and then try again.
>

I agree it is better to add information about this.

> 2)  I'm not sure if InvalidRepOriginId is correct here,  how about
> using OidIsValid in the below:
> +void
> +TwoPhaseTransactionGid(Oid subid, TransactionId xid, char *gid, int szgid)
> +{
> +       Assert(subid != InvalidRepOriginId);
>

I agree with this point but please note that this patch moves this
function so that it can be used from other places. Also, I think it is
wrong to use InvalidRepOriginId as we are passing here
subscription_oid, so, ideally, we should use InvalidOid but I would
rather prefer OidIsValid() as you suggested.


-- 
With Regards,
Amit Kapila.