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

2024-07-30 Thread Amit Kapila
On Tue, Jul 30, 2024 at 4:02 PM vignesh C wrote: > > On Thu, 25 Jul 2024 at 08:39, Amit Kapila wrote: > > > > On Wed, Jul 24, 2024 at 9:13 PM Tom Lane wrote: > > > > > > Amit Kapila writes: > > > > I merged these changes, made a few other cosmetic changes, and pushed > > > > the patch. > > >

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

2024-07-30 Thread vignesh C
On Thu, 25 Jul 2024 at 08:39, Amit Kapila wrote: > > On Wed, Jul 24, 2024 at 9:13 PM Tom Lane wrote: > > > > Amit Kapila writes: > > > I merged these changes, made a few other cosmetic changes, and pushed the > > > patch. > > > > There is a CF entry pointing at this thread [1]. Should it be

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

2024-07-24 Thread Amit Kapila
On Wed, Jul 24, 2024 at 9:13 PM Tom Lane wrote: > > Amit Kapila writes: > > I merged these changes, made a few other cosmetic changes, and pushed the > > patch. > > There is a CF entry pointing at this thread [1]. Should it be closed? > Yes, closed now. Thanks for the reminder. -- With

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

2024-07-24 Thread Tom Lane
Amit Kapila writes: > I merged these changes, made a few other cosmetic changes, and pushed the > patch. There is a CF entry pointing at this thread [1]. Should it be closed? regards, tom lane [1] https://commitfest.postgresql.org/48/4867/

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

2024-07-24 Thread Amit Kapila
On Tue, Jul 23, 2024 at 4:55 PM Amit Kapila wrote: > > On Mon, Jul 22, 2024 at 2:48 PM Peter Smith wrote: > > > > Hi, Patch v22-0001 LGTM apart from the following nitpicks > > > > I have included these in the attached. The patch looks good to me. I > am planning to push this tomorrow unless

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

2024-07-23 Thread Amit Kapila
On Mon, Jul 22, 2024 at 2:48 PM Peter Smith wrote: > > Hi, Patch v22-0001 LGTM apart from the following nitpicks > I have included these in the attached. The patch looks good to me. I am planning to push this tomorrow unless there are more comments. -- With Regards, Amit Kapila.

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

2024-07-22 Thread Peter Smith
Hi, Patch v22-0001 LGTM apart from the following nitpicks == src/sgml/ref/alter_subscription.sgml nitpick - /one needs to/you need to/ == src/backend/commands/subscriptioncmds.c CheckAlterSubOption: nitpick = "ideally we could have..." doesn't make sense because the code uses a more

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

2024-07-22 Thread Amit Kapila
On Mon, Jul 22, 2024 at 8:26 AM Hayato Kuroda (Fujitsu) wrote: > > ``` > @@ -1089,6 +1089,12 @@ CheckAlterSubOption(Subscription *sub, const char > *option, > * is because both failover and two_phase options of the slot on the > * publisher cannot be modified if the slot is currently

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

2024-07-21 Thread Amit Kapila
On Sat, Jul 20, 2024 at 9:31 PM vignesh C wrote: > > On Thu, 18 Jul 2024 at 07:41, Hayato Kuroda (Fujitsu) > wrote: > > > > Dear Peter, > > > > Thanks for giving comments! PSA new version. > > Couple of suggestions: > 1) How will user know which all transactions should be rolled back > since the

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

2024-07-21 Thread Hayato Kuroda (Fujitsu)
Dear Amit, > + /* > + * 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)

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

2024-07-20 Thread vignesh C
On Thu, 18 Jul 2024 at 07:41, Hayato Kuroda (Fujitsu) wrote: > > Dear Peter, > > Thanks for giving comments! PSA new version. Couple of suggestions: 1) How will user know which all transactions should be rolled back since the prepared transaction name will be different in subscriber like

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

2024-07-19 Thread Amit Kapila
On Thu, Jul 18, 2024 at 5:12 PM Amit Kapila 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

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

2024-07-19 Thread Amit Kapila
On Fri, Jul 19, 2024 at 10:45 AM Amit Kapila wrote: > > > == > > src/include/replication/slot.h > > > > 1. > > -extern void ReplicationSlotAlter(const char *name, bool failover); > > +extern void ReplicationSlotAlter(const char *name, bool *failover, > > + bool *two_phase); > > > > Use const?

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

2024-07-18 Thread Amit Kapila
On Fri, Jul 19, 2024 at 8:06 AM Peter Smith wrote: > > == > You wrote "tried to make the two_phase change before failover option > wherever it makes sense to keep the code consistent". But, still > failover is coded first in lots of places: > - libpqrcv_alter_slot > - ReplicationSlotAlter > -

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

2024-07-18 Thread Peter Smith
On Thu, Jul 18, 2024 at 9:42 PM Amit Kapila wrote: > ... > I agree and have done that in the attached. I have made some > additional changes: (a) removed the unrelated change of two_phase in > protocol.sgml, (b) tried to make the two_phase change before failover > option wherever it makes sense

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

2024-07-18 Thread Amit Kapila
On Thu, Jul 18, 2024 at 7:40 AM Hayato Kuroda (Fujitsu) wrote: > > Regarding the CheckAlterSubOption(), the ordering is still preserved > because I preferred to keep some specs. But I can agree that yours > make codes simpler. > It is better to simplify the code in this case. I have taken care

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

2024-07-17 Thread Zhijie Hou (Fujitsu)
On Thursday, July 18, 2024 10:11 AM Kuroda, Hayato/黒田 隼人 wrote: > > Dear Peter, > > Thanks for giving comments! PSA new version. I did a few more tests and analysis and didn't find issues. Just share the cases I tested: 1. After manually rolling back xacts for two_pc and switch two_pc option

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

2024-07-17 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for giving comments! PSA new version. I think most of comments were addressed, and I ran pgindent/pgperltidy again. Regarding the CheckAlterSubOption(), the ordering is still preserved because I preferred to keep some specs. But I can agree that yours make codes simpler. BTW,

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

2024-07-17 Thread Peter Smith
Hi Kuroda-San, here are some review comment for patch v19-1 == doc/src/sgml/ref/alter_subscription.sgml The previous patches have common failover/two_phase code checking for "Do not allow changing the option if the subscription is enabled", but it seems the docs were mentioning that only

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

2024-07-17 Thread Peter Smith
Hi, here are my review comments for patch v19-0002. == src/backend/commands/subscriptioncmds.c CheckAlterSubOption: nitpick - tweak some comment wording ~ On Wed, Jul 17, 2024 at 3:13 PM Hayato Kuroda (Fujitsu) wrote: > > > 1c. > > If the error checks can be moved to be done up-front,

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

2024-07-17 Thread Peter Smith
Hi, here are my review comments for v19-0001. == doc/src/sgml/protocol.sgml nitpick - Now there is >1 option. /The following option is supported:/The following options are supported:/ == src/backend/access/transam/twophase.c TwoPhaseTransactionGid: nitpick - renamed parameter

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

2024-07-16 Thread Hayato Kuroda (Fujitsu)
Dear Hou, Peter, Thanks for giving comments! PSA new version. Almost comments were addressed. What's new: 0001 - IsTwoPhaseTransactionGidForSubid() was updated per comment from Hou-san [1]. Some nitpicks were accepted. 0002 - An argument in CheckAlterSubOption() was renamed to "

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

2024-07-16 Thread Peter Smith
Hi, here is my review of the v18-0003 patch. == sgml/ref/alter_subscription.sgml nitpick - some minor tweaks to the documentation text. I also added a link back to the two_phase parameter. Please see the attached diffs file. == Kind Regards, Peter Smith. Fujitsu Australia diff --git

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

2024-07-16 Thread Peter Smith
Here are some review comments for patch v18-0002. == src/backend/commands/subscriptioncmds.c 1. CheckAlterSubOption 1a. It's not obvious why we are only checking the 'slot name' when needs_update==true, but OTOH is always checking the 'enabled' state. ~ 1b. Param 'needs_update' is a

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

2024-07-16 Thread Zhijie Hou (Fujitsu)
On Tuesday, July 16, 2024 1:17 PM Kuroda, Hayato/黒田 隼人 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

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

2024-07-16 Thread Peter Smith
Hi, here are some review comments for patch v18-0001. == doc/src/sgml/protocol.sgml nitpick - Although it is no fault of your patch, IMO it would be nicer for the TWO_PHASE description (of CREATE REPLICATION SLOT) to also be in the same consistent order as what you have (e.g. below

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

2024-07-15 Thread Hayato Kuroda (Fujitsu)
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. 0002: not so changed, just rebased. 0003: Typo was fixed, s/Alter/After/. [1]:

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

2024-07-15 Thread Amit Kapila
On Tue, Jul 9, 2024 at 6:23 PM Hayato Kuroda (Fujitsu) wrote: > > Previous patch set could not be accepted due to the initialization miss. > PSA new version. > Few minor comments: = 0001-patch 1. .git/rebase-apply/patch:253: space before tab in indent. errmsg("slot_name and

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

2024-07-13 Thread Zhijie Hou (Fujitsu)
On Tuesday, July 9, 2024 8:53 PM Hayato Kuroda (Fujitsu) wrote: > > > 0001 - Codes for SUBOPT_TWOPHASE_COMMIT are moved per requirement > [1]. > >Also, checks for failover and two_phase are unified into one > > function. > > 0002 - updated accordingly. An argument for the check

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

2024-07-09 Thread Hayato Kuroda (Fujitsu)
> 0001 - Codes for SUBOPT_TWOPHASE_COMMIT are moved per requirement [1]. >Also, checks for failover and two_phase are unified into one function. > 0002 - updated accordingly. An argument for the check function is added. > 0003 - this contains documentation changes required in [2].

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

2024-07-09 Thread Hayato Kuroda (Fujitsu)
Dear Amit, > > I see that in 0003/0004, the patch first aborts pending prepared > > transactions, update's catalog, and then change slot's property via > > walrcv_alter_slot. What if there is any ERROR (say the remote node is > > not reachable or there is an error while updating the catalog)

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

2024-07-09 Thread Hayato Kuroda (Fujitsu)
Dear Amit, Thanks for giving comments! Here I wanted to reply one of comments. > What should be the behavior if one tries to set slot_name to NONE and > also tries to toggle two_pahse option? You mentioned like below case, right? ``` ALTER SUBSCRIPTION sub SET (two_phase = false, slot_name =

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

2024-07-08 Thread Amit Kapila
On Mon, Jul 8, 2024 at 5:25 PM Amit Kapila wrote: > > > I see that in 0003/0004, the patch first aborts pending prepared > transactions, update's catalog, and then change slot's property via > walrcv_alter_slot. What if there is any ERROR (say the remote node is > not reachable or there is an

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

2024-07-08 Thread Amit Kapila
On Mon, Jul 8, 2024 at 12:34 PM Hayato Kuroda (Fujitsu) wrote: > > > Another possible problem is related to my use case. I haven't reproduced > > this > > case, just some thoughts. I guess, when two_phase is ON, the PREPARE > > statement > > may be truncated from the WAL at checkpoint, but

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

2024-07-05 Thread Vitaly Davydov
Hi Kuroda-san, Thank you very much for the patch. In general, it seem to work well for me, but there seems to be a memory access problem in libpqrcv_alter_slot -> quote_identifier in case of NULL slot_name. It happens, if the two_phase option is altered on a subscription without slot. I

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

2024-07-05 Thread Hayato Kuroda (Fujitsu)
Dear Amit, Sorry, I forgot to say one content. > > 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

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

2024-07-04 Thread Amit Kapila
On Thu, Jul 4, 2024 at 1:34 PM Hayato Kuroda (Fujitsu) 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); > >

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

2024-07-04 Thread Amit Kapila
On Mon, Apr 22, 2024 at 2:26 PM Hayato Kuroda (Fujitsu) 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

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

2024-06-25 Thread Hayato Kuroda (Fujitsu)
> Dear hackers, > > I found that v12 patch set could not be accepted by the cfbot. PSA new > version. To make others more trackable, I shared changes just in case. All failures were occurred on the pg_dump code. I added an attribute in pg_subscription and modified pg_dump code, but it was

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

2024-05-16 Thread Peter Smith
Hi Kuroda-san, I did not apply these v12* patches, but I have diff'ed all of them with the previous v11* patches and confirmed that all of my previous review comments now seem to be addressed. I don't have any more comments to make at this time. Thanks! == Kind Regards, Peter Smith. Fujitsu

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

2024-05-16 Thread Peter Smith
Hi, Here are my remaining review comments for the latest v11* patches. // v11-0001 // No changes. No comments. // v11-0002 // == doc/src/sgml/ref/alter_subscription.sgml 2.1. ALTER SUBSCRIPTION ... SET (failover = on|off) and - ALTER SUBSCRIPTION ...

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

2024-05-15 Thread Hayato Kuroda (Fujitsu)
Dear Peter, > You wrote "Fixed" for that patch v9-0004 suggestion but I don't think > anything was changed at all. Accidentally missed? Sorry, I missed to do `git add` after the revision. The change was also included in new patch [1]. [1]:

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

2024-05-14 Thread Peter Smith
On Tue, May 14, 2024 at 10:03 PM Hayato Kuroda (Fujitsu) wrote: > > Dear Peter, > ... > > 4.11. > > > > +# Alter the two_phase with the force_alter option. Apart from the the last > > +# ALTER SUBSCRIPTION command, the command will abort the prepared > > transaction > > +# and succeed. > > > >

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

2024-05-14 Thread Peter Smith
Hi Kuroda-san. Here are my review comments for latest v10* patches. // patch v10-0001 // No changes. No comments. // patch v10-0002 // == Commit message 2.1. Regarding the false->true case, the backend process alters the subtwophase to

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

2024-05-14 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for reviewing! New patch is available in [1]. > I'm having second thoughts about how these patches mention the option > values "on|off". These are used in the ALTER SUBSCRIPTION document > page for 'two_phase' and 'failover' parameters, and then those > "on|off" get propagated

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

2024-05-13 Thread Peter Smith
Hi Kuroda-san, Here are some review comments for all patches v9* // Patch v9-0001 // There were no changes since v8-0001, so no comments. // Patch v9-0002 // == Commit Message 2.1. Regarding the off->on case, the logical replication already has a mechanism

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

2024-05-13 Thread Peter Smith
Hi Kuroda-san, I'm having second thoughts about how these patches mention the option values "on|off". These are used in the ALTER SUBSCRIPTION document page for 'two_phase' and 'failover' parameters, and then those "on|off" get propagated to the code comments, error messages, and tests... Now I

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

2024-05-13 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for giving comments! New patch was posted in [1]. > 0.1 General - Patch name > > /SUBSCIRPTION/SUBSCRIPTION/ Fixed. > == > 0.2 General - Apply > > FYI, there are whitespace warnings: > > git > apply ../patches_misc/v8-0004-Add-force_alter-option-for-ALTER-SUBSCIRPTI >

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

2024-05-13 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for reviewing! Patch can be available in [1]. > == > src/sgml/ref/alter_subscription.sgml > > 1. > + > + The two_phase parameter can only be altered when > the > + subscription is disabled. When altering the parameter from > on > + to off, the backend

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

2024-05-13 Thread Peter Smith
Hi, Here are some review comments for v8-0002. == Commit message 1. Regarding the off->on case, the logical replication already has a mechanism for it, so there is no need to do anything special for the on->off case; however, we must connect to the publisher and expressly change the

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

2024-05-13 Thread Peter Smith
Hi, Here are some comments for v8-0004 == 0.1 General - Patch name /SUBSCIRPTION/SUBSCRIPTION/ == 0.2 General - Apply FYI, there are whitespace warnings: git apply ../patches_misc/v8-0004-Add-force_alter-option-for-ALTER-SUBSCIRPTION-.-S.patch

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

2024-05-13 Thread Peter Smith
Hi, Here are some review comments for v8-0003. == src/sgml/ref/alter_subscription.sgml 1. + + The two_phase parameter can only be altered when the + subscription is disabled. When altering the parameter from on + to off, the backend process checks prepared +

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

2024-05-09 Thread Hayato Kuroda (Fujitsu)
Dear Peter, > > == > Commit message > > 1. > A detailed commit message is needed to describe the purpose and > details of this patch. Added. > == > doc/src/sgml/ref/alter_subscription.sgml > > 2. CREATE SUBSCRIPTION > > Shouldn't there be an entry for "force_alter" parameter in the

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

2024-05-09 Thread Hayato Kuroda (Fujitsu)
Dear Peter, > Commit Message > > 1. > The patch needs a commit message to describe the purpose and highlight > any limitations and other details. Added. > == > doc/src/sgml/ref/alter_subscription.sgml > > 2. > + > + > + The two_phase parameter can only be altered when > the > +

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

2024-05-09 Thread Hayato Kuroda (Fujitsu)
Dear Peter, > == > Commit message > > 1. > IIUC there is quite a lot of subtlety and details about why the slot > option needs to be changed only when altering "true" to "false", but > not when altering "false" to "true". > > It also should explain why PreventInTransactionBlock is only

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

2024-05-09 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for reviewing! The patch will be posted in the upcoming post. > == > src/backend/access/transam/twophase.c > > 1. IsTwoPhaseTransactionGidForSubid > > +/* > + * IsTwoPhaseTransactionGidForSubid > + * Check whether the given GID is formed by TwoPhaseTransactionGid. > + */

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

2024-05-09 Thread Peter Smith
Hi, Here are some review comments for patch v7-0004 == Commit message 1. A detailed commit message is needed to describe the purpose and details of this patch. == doc/src/sgml/ref/alter_subscription.sgml 2. CREATE SUBSCRIPTION Shouldn't there be an entry for "force_alter" parameter in

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

2024-05-09 Thread Peter Smith
Hi, Here are some review comments for the patch v7-0003. == Commit Message 1. The patch needs a commit message to describe the purpose and highlight any limitations and other details. == doc/src/sgml/ref/alter_subscription.sgml 2. + + + The two_phase parameter can only be

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

2024-05-08 Thread Peter Smith
Hi, Here are some review comments for v7-0002 == Commit message 1. IIUC there is quite a lot of subtlety and details about why the slot option needs to be changed only when altering "true" to "false", but not when altering "false" to "true". It also should explain why

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

2024-05-08 Thread Peter Smith
Hi Kuroda-san, Thanks for addressing most of my v6-0001 review comments. Below are some minor follow-up comments for v7-0001. == src/backend/access/transam/twophase.c 1. IsTwoPhaseTransactionGidForSubid +/* + * IsTwoPhaseTransactionGidForSubid + * Check whether the given GID is formed by

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

2024-05-08 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for reviewing! Here are updated patches. I updated patches only for HEAD. > == > Commit message > > 1. > This patch allows user to alter two_phase option > > /allows user/allows the user/ > > /to alter two_phase option/to alter the 'two_phase' option/ Fixed. > ==

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

2024-05-07 Thread Peter Smith
Hi, Here are some review comments for patch v6-0001 == Commit message 1. This patch allows user to alter two_phase option /allows user/allows the user/ /to alter two_phase option/to alter the 'two_phase' option/ == doc/src/sgml/ref/alter_subscription.sgml 2. two_phase can be altered

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

2024-04-23 Thread Hayato Kuroda (Fujitsu)
Dear hackers, Per recent commit (b29cbd3da), our patch needed to be rebased. Here is an updated version. Best Regards, Hayato Kuroda FUJITSU LIMITED https://www.fujitsu.com/ v6-0001-Allow-altering-of-two_phase-option-of-a-SUBSCRIPT.patch Description:

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

2024-04-22 Thread Vitaly Davydov
Dear Hayato, On Monday, April 22, 2024 15:54 MSK, "Hayato Kuroda (Fujitsu)" wrote:  > Dear Vitaly, > > > I looked at the patch and realized that I can't try it easily in the near > > future > > because the solution I'm working on is based on PG16 or earlier. This patch > > is > > not easily

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

2024-04-22 Thread Hayato Kuroda (Fujitsu)
> Dear Vitaly, > > > I looked at the patch and realized that I can't try it easily in the near > > future > > because the solution I'm working on is based on PG16 or earlier. This patch > > is > > not easily applicable to the older releases. I have to port my solution to > > the > > master,

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

2024-04-22 Thread Hayato Kuroda (Fujitsu)
Dear hackers, > Looking at this a bit more, maybe rolling back all prepared transactions on > the > subscriber when toggling two_phase from true to false might not be desirable > for the customer. Maybe we should have an option for customers to control > whether transactions should be rolled

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

2024-04-22 Thread Hayato Kuroda (Fujitsu)
Dear Vitaly, > I looked at the patch and realized that I can't try it easily in the near > future > because the solution I'm working on is based on PG16 or earlier. This patch is > not easily applicable to the older releases. I have to port my solution to the > master, which is not done yet. We

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

2024-04-22 Thread Ajin Cherian
On Thu, Apr 18, 2024 at 4:26 PM Ajin Cherian wrote: > > Attaching the patch for your review and comments. Big thanks to Kuroda-san > for also working on the patch. > > Looking at this a bit more, maybe rolling back all prepared transactions on the subscriber when toggling two_phase from true to

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

2024-04-18 Thread Ajin Cherian
On Tue, Apr 16, 2024 at 4:25 PM Amit Kapila wrote: > > > > > Can you please once consider the idea shared by me at [1] (One naive > idea is that on the publisher .) to solve this problem? > > [1] - >

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

2024-04-16 Thread Ajin Cherian
On Tue, Apr 16, 2024 at 1:31 AM Давыдов Виталий wrote: > Dear All, > Just interested, does anyone tried to reproduce the problem with slow > catchup of twophase transactions (pgbench should be used with big number of > clients)? I haven't seen any messages from anyone other that me that the >

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

2024-04-16 Thread Amit Kapila
On Tue, Apr 16, 2024 at 7:48 AM Hayato Kuroda (Fujitsu) wrote: > > > > FYI - We also considered the idea which walsender waits until all prepared > > transactions > > > are resolved before decoding and sending changes, but it did not work well > > > - the restarted walsender sent only COMMIT

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

2024-04-15 Thread Hayato Kuroda (Fujitsu)
Dear Amit, > > FYI - We also considered the idea which walsender waits until all prepared > transactions > > are resolved before decoding and sending changes, but it did not work well > > - the restarted walsender sent only COMMIT PREPARED record for > transactions which > > have been prepared

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

2024-04-15 Thread Давыдов Виталий
Dear All, On Wednesday, April 10, 2024 17:16 MSK, Давыдов Виталий wrote:  Hi Amit, Ajin, All Thank you for the patch and the responses. I apologize for my delayed answer due to some curcumstances. On Wednesday, April 10, 2024 14:18 MSK, Amit Kapila wrote: Vitaly, does the minimal solution

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

2024-04-15 Thread Amit Kapila
On Mon, Apr 15, 2024 at 1:28 PM Hayato Kuroda (Fujitsu) wrote: > > > Vitaly, does the minimal solution provided by the proposed patch > > (Allow to alter two_phase option of a subscriber provided no > > uncommitted > > prepared transactions are pending on that subscription.) address your use > >

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

2024-04-15 Thread Hayato Kuroda (Fujitsu)
Dear Amit, > Vitaly, does the minimal solution provided by the proposed patch > (Allow to alter two_phase option of a subscriber provided no > uncommitted > prepared transactions are pending on that subscription.) address your use > case? I think we do not have to handle cases which there are

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

2024-04-10 Thread Hayato Kuroda (Fujitsu)
Dear Amit, > One naive idea is that on the publisher we can remember whether the > prepare has been sent and if so then only send commit_prepared, > otherwise send the entire transaction. On the subscriber-side, we > somehow, need to ensure before applying the first change whether the >

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

2024-04-10 Thread Давыдов Виталий
Hi Amit, Ajin, All Thank you for the patch and the responses. I apologize for my delayed answer due to some curcumstances. On Wednesday, April 10, 2024 14:18 MSK, Amit Kapila wrote: Vitaly, does the minimal solution provided by the proposed patch (Allow to alter two_phase option of a

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

2024-04-10 Thread Amit Kapila
On Fri, Apr 5, 2024 at 4:59 PM Ajin Cherian wrote: > > On Thu, Apr 4, 2024 at 4:38 PM Amit Kapila wrote: >> >> >> I think this would probably be better than the current situation but >> can we think of a solution to allow toggling the value of two_phase >> even when prepared transactions are

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

2024-04-05 Thread Ajin Cherian
On Thu, Apr 4, 2024 at 4:38 PM Amit Kapila wrote: > > I think this would probably be better than the current situation but > can we think of a solution to allow toggling the value of two_phase > even when prepared transactions are present? Can you please summarize > the reason for the problems

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

2024-04-03 Thread Amit Kapila
On Thu, Apr 4, 2024 at 10:53 AM Ajin Cherian wrote: > > On Wed, Mar 6, 2024 at 1:29 AM Давыдов Виталий > wrote: >> >> In usual work, the subscription has two_phase = on. I have to change this >> option at catchup stage only, but this parameter can not be altered. There >> was a patch proposal

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

2024-04-03 Thread Ajin Cherian
On Wed, Mar 6, 2024 at 1:29 AM Давыдов Виталий wrote: > In usual work, the subscription has two_phase = on. I have to change this > option at catchup stage only, but this parameter can not be altered. There > was a patch proposal in past to implement altering of two_phase option, but > it was

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

2024-03-06 Thread Amit Kapila
On Tue, Mar 5, 2024 at 7:59 PM Давыдов Виталий wrote: > > Thank you for the reply. > > On Tuesday, March 05, 2024 12:05 MSK, Heikki Linnakangas > wrote: > > > In a nutshell, this changes PREPARE TRANSACTION so that if > synchronous_commit is 'off', the PREPARE TRANSACTION is not fsync'd to >

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

2024-03-05 Thread Давыдов Виталий
Hi Heikki, Thank you for the reply. On Tuesday, March 05, 2024 12:05 MSK, Heikki Linnakangas wrote:  In a nutshell, this changes PREPARE TRANSACTION so that if synchronous_commit is 'off', the PREPARE TRANSACTION is not fsync'd to disk. So if you crash after the PREPARE TRANSACTION has

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

2024-03-05 Thread Heikki Linnakangas
On 29/02/2024 19:34, Давыдов Виталий wrote: Dear All, Consider, please, my patch for async commit for twophase transactions. It can be applicable when catchup performance is not enought with publication parameter twophase = on. The key changes are: * Use XLogSetAsyncXactLSN instead of

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

2024-02-29 Thread Давыдов Виталий
Dear All, Consider, please, my patch for async commit for twophase transactions. It can be applicable when catchup performance is not enought with publication parameter twophase = on. The key changes are: * Use XLogSetAsyncXactLSN instead of XLogFlush as it is for usual transactions. * In

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

2024-02-27 Thread Давыдов Виталий
Hi Amit, On Tuesday, February 27, 2024 16:00 MSK, Amit Kapila wrote: As we do XLogFlush() at the time of prepare then why it is not available? OR are you talking about this state after your idea/patch where you are trying to make both Prepare and Commit_prepared records async?Right, I'm

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

2024-02-27 Thread Amit Kapila
On Tue, Feb 27, 2024 at 4:49 PM Давыдов Виталий wrote: > > Thank you for your interest in the discussion! > > On Monday, February 26, 2024 16:24 MSK, Amit Kapila > wrote: > > > I think the reason is probably that when the WAL record for prepared is > already flushed then what will be the idea

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

2024-02-27 Thread Давыдов Виталий
Hi Amit, Thank you for your interest in the discussion! On Monday, February 26, 2024 16:24 MSK, Amit Kapila wrote:   I think the reason is probably that when the WAL record for prepared is already flushed then what will be the idea of async commit here?I think, the idea of async commit

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

2024-02-26 Thread Amit Kapila
On Fri, Feb 23, 2024 at 10:41 PM Давыдов Виталий wrote: > > Amit Kapila wrote: > > I don't see we do anything specific for 2PC transactions to make them behave > differently than regular transactions with respect to synchronous_commit > setting. What makes you think so? Can you pin point the

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

2024-02-23 Thread Давыдов Виталий
Hi Amit, Amit Kapila wrote: I don't see we do anything specific for 2PC transactions to make them behave differently than regular transactions with respect to synchronous_commit setting. What makes you think so? Can you pin point the code you are referring to?Yes, sure. The function

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

2024-02-23 Thread Давыдов Виталий
Hi Ajin, Thank you for your feedback. Could you please try to increase the number of clients (-c pgbench option) up to 20 or more? It seems, I forgot to specify it. With best regards, Vitaly Davydov On Fri, Feb 23, 2024 at 12:29 AM Давыдов Виталий wrote: Dear All, I'd like to present and

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

2024-02-22 Thread Ajin Cherian
On Fri, Feb 23, 2024 at 12:29 AM Давыдов Виталий wrote: > Dear All, > > I'd like to present and talk about a problem when 2PC transactions are > applied quite slowly on a replica during logical replication. There is a > master and a replica with established logical replication from the master >

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

2024-02-22 Thread Amit Kapila
On Thu, Feb 22, 2024 at 6:59 PM Давыдов Виталий wrote: > > I'd like to present and talk about a problem when 2PC transactions are > applied quite slowly on a replica during logical replication. There is a > master and a replica with established logical replication from the master to > the