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: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: 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-19T12:44:55Z
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 Thu, Jul 18, 2024 at 5:12 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> I'll continue my review and testing of the patch but I thought of
> sharing what I have done till now.
>

+ /*
+ * Do not allow changing the option if the subscription is enabled. This
+ * is because both failover and two_phase options of the slot on the
+ * publisher cannot be modified if the slot is currently acquired by the
+ * existing walsender.
+ */
+ if (sub->enabled)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot set %s for enabled subscription",
+ option)));

As per my understanding, the above comment is not true when we are
changing 'two_phase' option from 'false' to 'true' because in that
case, the existing walsender will only change it. So, ideally, we can
allow toggling two_phase from 'false' to 'true' without the above
restriction.

If this is correct then we don't even need to error for the case
"cannot alter two_phase when logical replication worker is still
running" when 'two_phase' option is changed from 'false' to 'true'.

Now, assuming the above observations are correct, we may still want to
have the same behavior when toggling two_phase option but we can at
least note down that in the comments so that if required the same can
be changed when toggling 'two_phase' option from 'false' to 'true' in
future.

Thoughts?

-- 
With Regards,
Amit Kapila.