public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Давыдов Виталий <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Slow catchup of 2PC (twophase) transactions on replica in LR
Date: Thu, 4 Jul 2024 17:01:40 +0530
Message-ID: <CAA4eK1LhYsNq_0XKspfWKx4oXZpXk_q-A1Oa-tbA-wqQUudhCQ@mail.gmail.com> (raw)
In-Reply-To: <OSBPR01MB2552A7C652D452185785A3C2F5DE2@OSBPR01MB2552.jpnprd01.prod.outlook.com>
References: <8fab8-65d74c80-1-2f28e880@39088166>
<CAA4eK1KOs3s6syZqUgrd2WvjTz64SGf0ToZcRoPMCKKH+M0YFQ@mail.gmail.com>
<99df2-65d8d200-1-63c26880@172687752>
<CAA4eK1KtjjPxe5tjWeGNf5CfF9qfceVQCCC9XGsKzR-ifQD9uA@mail.gmail.com>
<ba4b6-65ddc580-9-5c3e3080@134362348>
<CAA4eK1J6wOMkDDt3iOs+JqkvQvuwSORosTpqdejgvXqFmxex=A@mail.gmail.com>
<badcf-65dde500-3-24ba3c80@28790566>
<cedbe-65e0c000-1-6db17700@133269862>
<[email protected]>
<f9f1b-65e72c80-7-533dda00@76203995>
<CAFPTHDbU4EkVmKL66i5BzrmQ6LFftQPx_TZ1euKt9fvVzQDyjA@mail.gmail.com>
<CAA4eK1+n7M2S1OpoGWDd+YZkDCuURMdVRbvP0eELQUvWgmDneg@mail.gmail.com>
<CAFPTHDa=pJSZ_4dV5DPAOapRSgPcyyUTP0WzGY2Rz_D3-gwraw@mail.gmail.com>
<CAA4eK1K1fSkeK=kc26G5cq87vQG4=1qs_b+no4+ep654SeBy1w@mail.gmail.com>
<OSBPR01MB2552707A847936E6803CFAA5F5092@OSBPR01MB2552.jpnprd01.prod.outlook.com>
<CAA4eK1KY=uwXXuVMtuNTYHGFbbXgDveoFoP3UbxNXqxCAx8GBQ@mail.gmail.com>
<OSBPR01MB25528F4B0B8178D3AA8DE2BFF5082@OSBPR01MB2552.jpnprd01.prod.outlook.com>
<CAA4eK1KNrm8xE-ZOAS249m+DOk=YQ_YaoM-GsZEvC8rTMtdW8w@mail.gmail.com>
<CAFPTHDaz4cyRN3NbjYtkqCKFj7Dv1u0Z8wviY8fnJW4+0CkF_Q@mail.gmail.com>
<CAFPTHDZNwMWfJZprw9jsoa4-19-h337KRY9Pt41PdNiQTTYaiQ@mail.gmail.com>
<OSBPR01MB2552083E59F366B465DF0EF8F5122@OSBPR01MB2552.jpnprd01.prod.outlook.com>
<CAA4eK1Jpj2Ebk2C_9meEDfZKen38cxpzOkxZ7818f1cfsT8=3w@mail.gmail.com>
<OSBPR01MB2552A7C652D452185785A3C2F5DE2@OSBPR01MB2552.jpnprd01.prod.outlook.com>
On Thu, Jul 4, 2024 at 1:34 PM Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
>
> > >
> > > 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
>
Is it possible to get a NOTICE instead of a WARNING?
>
> > 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);
> ```
>
But that is not a good reason for this operation to stop workers
first. Instead, we should prohibit this operation if any worker is
present. The reason is that there is always a chance that if any
worker is alive, it can prepare a new transaction after we have
checked for the presence of any prepared transactions.
Comments:
=========
1.
There is no need to do something remarkable regarding
+ * the "false" to "true" case; the backend process alters
+ * subtwophase <funny_char> to LOGICALREP_TWOPHASE_STATE_PENDING once.
+ * After the subscription is enabled, a new logical
+ * replication worker requests to change the two_phase
+ * option of its slot from pending to true when the
+ * initial data synchronization is done. The code path is
+ * the same as the case in which two_phase <funny_char> is initially
+ * set <funny_char> to true.
The patch has some funny characters in the above comment at the places
highlighted by me. It seems you have copied from some editor that has
inserted such characters.
2.
/*
* Do not allow toggling of two_phase option. Doing so could cause
* missing of transactions and lead to an inconsistent replica.
* See comments atop worker.c
*
* Note: Unsupported twophase indicates that this call originated
* from AlterSubscription.
*/
if (!IsSet(supported_opts, SUBOPT_TWOPHASE_COMMIT))
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("unrecognized subscription parameter: \"%s\"", defel->defname)));
This part of the code must either be removed or converted to an assert.
3. The tests added in 099_twophase_added.pl should be part of 021_twophase.pl
--
With Regards,
Amit Kapila.
view thread (11+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Slow catchup of 2PC (twophase) transactions on replica in LR
In-Reply-To: <CAA4eK1LhYsNq_0XKspfWKx4oXZpXk_q-A1Oa-tbA-wqQUudhCQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox