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

From: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Vitaly Davydov <v(dot)davydov(at)postgrespro(dot)ru>, Ajin Cherian <itsajin(at)gmail(dot)com>
Subject: RE: Slow catchup of 2PC (twophase) transactions on replica in LR
Date: 2024-07-17 00:58:51
Message-ID: OS3PR01MB571834FBD3E6D3804484038F94A32@OS3PR01MB5718.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tuesday, July 16, 2024 1:17 PM Kuroda, Hayato/黒田 隼人 <kuroda(dot)hayato(at)fujitsu(dot)com> wrote
>
> Dear Amit, Hou,
>
> Thanks for giving comments! PSA new versions.
> What's new:
>
> 0001: included Hou's patch [1] not to overwrite slot options.
> Some other comments were also addressed.

Thanks for the patch!

One more issue I found is that:

+IsTwoPhaseTransactionGidForSubid(Oid subid, char *gid)
+{
+ int ret;
+ Oid subid_written;
+ TransactionId xid;
+
+ ret = sscanf(gid, "pg_gid_%u_%u", &subid_written, &xid);
+
+ return (ret == 2 && subid == subid_written);

I think it's not correct to use sscanf here, because it will return the same value
even if the gid is "pg_gid_123_123_123_123..." which isn't a
gid created by the apply worker. I think we should use TwoPhaseTransactionGid
to build the gid string and compare it with each existing gid(strcmp).

Best Regards,
Hou zj

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Joseph Koshakow 2024-07-17 01:23:27 Re: Remove dependence on integer wrapping
Previous Message Paul George 2024-07-17 00:50:15 Re: Wrong results with grouping sets