Horton

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

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

View thread
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Peter Smith' <smithpb2250@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>, Ajin Cherian <itsajin@gmail.com>, Давыдов Виталий <v.davydov@postgrespro.ru>
Date: 2024-05-17T03:06:34Z
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 Peter,

Thanks for reviewing! Here are new patches.

> ======
> doc/src/sgml/ref/alter_subscription.sgml
> 
> 2.1.
>     <command>ALTER SUBSCRIPTION ... SET (failover = on|off)</command>
> and
> -   <command>ALTER SUBSCRIPTION ... SET (two_phase =
> on|off)</command>
> +   <command>ALTER SUBSCRIPTION ... SET (two_phase = off)</command>
> 
> My other thread patch has already been pushed [1], so now you can
> modify this to say "true|false" as previously suggested.

Fixed accordingly.

> //////////
> v11-0003
> //////////
> 
> ======
> src/backend/commands/subscriptioncmds.c
> 
> 3.1. AlterSubscription
> 
> + subtwophase = LOGICALREP_TWOPHASE_STATE_DISABLED;
> + }
> + else
> + subtwophase = LOGICALREP_TWOPHASE_STATE_PENDING;
> +
> +
>   /* Change system catalog acoordingly */
>   values[Anum_pg_subscription_subtwophasestate - 1] =
> - CharGetDatum(opts.twophase ?
> - LOGICALREP_TWOPHASE_STATE_PENDING :
> - LOGICALREP_TWOPHASE_STATE_DISABLED);
> + CharGetDatum(subtwophase);
>   replaces[Anum_pg_subscription_subtwophasestate - 1] = true;
> 
> Sorry, I don't think that 'subtwophase' change is an improvement --
> IMO the existing ternary code was fine as-is.
> 
> I only reported the excessive flag checking in the previous v10-0003
> review because of some extra "if (!opts.twophase)" code but that was
> caused by what you called "wrong git operations." and is already fixed
> now.

Ok, the part was reverted.

> //////////
> v11-0004
> //////////
> 
> ======
> src/sgml/catalogs.sgml
> 
> 4.1.
> +     <row>
> +      <entry role="catalog_table_entry"><para role="column_definition">
> +       <structfield>subforcealter</structfield> <type>bool</type>
> +      </para>
> +      <para>
> +       If true, then the <link
> linkend="sql-altersubscription"><command>ALTER
> SUBSCRIPTION</command></link>
> +       can disable <literal>two_phase</literal> option, even if there are
> +       uncommitted prepared transactions from when
> <literal>two_phase</literal>
> +       was enabled
> +      </para></entry>
> +     </row>
> +
> 
> I think this description should be changed to say what it *really*
> does. IMO, the stuff about 'two_phase' is more like a side-effect.
> 
> SUGGESTION (this is just pseudo-code. You can add the CREATE
> SUBSCRIPTION 'force_alter' link appropriately)
> 
> If true, then the <command>ALTER SUBSCRIPTION</command> command can
> sometimes be forced to proceed instead of giving an error. See
> <link>force_alter</link> parameter for details about when this might
> be useful.
>

Fixed. But One point, the word "command" was removed. I checked other parts and
it seemed not to be needed.

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