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

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Давыдов Виталий <v.davydov@postgrespro.ru>, Heikki Linnakangas <hlinnaka@iki.fi>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2024-07-04T08:04:47Z
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

Attachments

Dear Amit,

> >
> > It succeeds if force_alter is also expressly set. Prepared transactions will be
> > aborted at that time.
> >
> > ```
> > subscriber=# ALTER SUBSCRIPTION sub SET (two_phase = off, force_alter =
> on);
> > ALTER SUBSCRIPTION
> >
> 
> Isn't it better to give a Notice when force_alter option leads to the
> rollback of already prepared transactions?

Indeed. I think this can be added for 0003. For now, it says like:

```
postgres=# ALTER SUBSCRIPTION sub SET (TWO_PHASE = off, FORCE_ALTER = on);
WARNING:  requested altering to two_phase = false but there are prepared transactions done by the subscription
DETAIL:  Such transactions are being rollbacked.
ALTER SUBSCRIPTION
```

> I have another question on the latest 0001 patch:
> + /*
> + * Stop all the subscription workers, just in case.
> + * Workers may still survive even if the subscription is
> + * disabled.
> + */
> + logicalrep_workers_stop(subid);
> 
> In which case the workers will survive when the subscription is disabled?

I think both normal and tablesync worker can survive, because ALTER SUBSCRIPTION
DISABLE command does not send signal to workers. It just change the system catalog.
logicalrep_workers_stop() is added to ensure all workers are stopped.

Actually, earlier version (-v3) did not have a mechanism but they sometimes got
assertion failures in maybe_reread_subscription(). This was because the survived
workers read pg_subscription catalog and failed below assertion:

```
	/* two-phase cannot be altered while the worker exists */
	Assert(newsub->twophasestate == MySubscription->twophasestate);
```

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/