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

On Sat, Jul 20, 2024 at 9:31 PM vignesh C  wrote:
>
> On Thu, 18 Jul 2024 at 07:41, Hayato Kuroda (Fujitsu)
>  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  linkend="sql-createsubscription-params-with-two-phase">two_phase
> +      from true to false, the backend
> +      process reports and an error if any prepared transactions done by the
> +      logical replication worker (from when two_phase
> +      parameter was still true) 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.