Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-17 Thread Ajin Cherian
On Tue, Jun 15, 2021 at 5:34 PM Ajin Cherian wrote: Since we've decided to not commit this for PG-14, I've added these patches along with the larger patch-set for subscriber side 2pc in thread [1] [1] -

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-15 Thread Ajin Cherian
On Fri, Jun 11, 2021 at 8:14 PM Amit Kapila wrote: > > Also, I can take care of the below cosmetic issues before committing > if we decide to do this for PG-14. > > Few cosmetic issues: > == > 1. git diff --check shows > src/bin/pg_basebackup/t/030_pg_recvlogical.pl:109: new

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-12 Thread Amit Kapila
On Sat, Jun 12, 2021 at 12:56 AM Jeff Davis wrote: > > On Fri, 2021-06-11 at 15:43 +0530, Amit Kapila wrote: > > The new patches look mostly good apart from the below cosmetic > > issues. > > I think the question is whether we want to do these for PG-14 or > > postpone them till PG-15. I think

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-11 Thread Jeff Davis
On Fri, 2021-06-11 at 15:43 +0530, Amit Kapila wrote: > The new patches look mostly good apart from the below cosmetic > issues. > I think the question is whether we want to do these for PG-14 or > postpone them till PG-15. I think these don't appear to be risky > changes so we can get them in

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-11 Thread Ajin Cherian
On Fri, Jun 11, 2021 at 8:14 PM Amit Kapila wrote: > > On Thu, Jun 10, 2021 at 2:04 PM Ajin Cherian wrote: > > > > The new patches look mostly good apart from the below cosmetic issues. > I think the question is whether we want to do these for PG-14 or > postpone them till PG-15. I think these

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-11 Thread Amit Kapila
On Thu, Jun 10, 2021 at 2:04 PM Ajin Cherian wrote: > The new patches look mostly good apart from the below cosmetic issues. I think the question is whether we want to do these for PG-14 or postpone them till PG-15. I think these don't appear to be risky changes so we can get them in PG-14 as

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-10 Thread Ajin Cherian
On Wed, Jun 9, 2021 at 9:57 PM Amit Kapila wrote: > > On Wed, Jun 9, 2021 at 4:50 PM Amit Kapila wrote: > > > > On Wed, Jun 9, 2021 at 1:53 AM Jeff Davis wrote: > > > > > > On Tue, 2021-06-08 at 17:41 +1000, Ajin Cherian wrote: > > > > Here's an updated patchset that adds back in the option for

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Amit Kapila
On Thu, Jun 10, 2021 at 4:13 AM Jeff Davis wrote: > > On Wed, 2021-06-09 at 17:27 +0530, Amit Kapila wrote: > > 2. In the main patch [1], we do send two_phase option even during > > START_REPLICATION for the very first time when the two_phase can be > > enabled. There are reasons as described in

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Tom Lane
Jeff Davis writes: > On Wed, 2021-06-09 at 16:50 +0530, Amit Kapila wrote: >> BTW, can't we consider it to be part of >> create_slot_opt_list? > Yes, that would be better. It looks like the physical and logical slot > options are mixed together -- should they be separated in the grammar > so

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Jeff Davis
On Wed, 2021-06-09 at 17:27 +0530, Amit Kapila wrote: > 2. In the main patch [1], we do send two_phase option even during > START_REPLICATION for the very first time when the two_phase can be > enabled. There are reasons as described in the worker.c why we can't > enable it during

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Jeff Davis
On Wed, 2021-06-09 at 16:50 +0530, Amit Kapila wrote: > BTW, can't we consider it to be part of > create_slot_opt_list? Yes, that would be better. It looks like the physical and logical slot options are mixed together -- should they be separated in the grammar so that using an option with the

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Amit Kapila
On Wed, Jun 9, 2021 at 4:50 PM Amit Kapila wrote: > > On Wed, Jun 9, 2021 at 1:53 AM Jeff Davis wrote: > > > > On Tue, 2021-06-08 at 17:41 +1000, Ajin Cherian wrote: > > > Here's an updated patchset that adds back in the option for two-phase > > > in CREATE_REPLICATION_SLOT command and a second

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Amit Kapila
On Wed, Jun 9, 2021 at 1:53 AM Jeff Davis wrote: > > On Tue, 2021-06-08 at 17:41 +1000, Ajin Cherian wrote: > > Here's an updated patchset that adds back in the option for two-phase > > in CREATE_REPLICATION_SLOT command and a second patch that adds > > support for > > two-phase decoding in

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-09 Thread Ajin Cherian
On Wed, Jun 9, 2021 at 6:23 AM Jeff Davis wrote: > > On Tue, 2021-06-08 at 17:41 +1000, Ajin Cherian wrote: > > Here's an updated patchset that adds back in the option for two-phase > > in CREATE_REPLICATION_SLOT command and a second patch that adds > > support for > > two-phase decoding in

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-08 Thread Jeff Davis
On Tue, 2021-06-08 at 17:41 +1000, Ajin Cherian wrote: > Here's an updated patchset that adds back in the option for two-phase > in CREATE_REPLICATION_SLOT command and a second patch that adds > support for > two-phase decoding in pg_recvlogical. A few things: * I suggest putting the TWO_PHASE

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-08 Thread Ajin Cherian
On Mon, Jun 7, 2021 at 3:17 PM Amit Kapila wrote: > Pushed the above patch. Here's an updated patchset that adds back in the option for two-phase in CREATE_REPLICATION_SLOT command and a second patch that adds support for two-phase decoding in pg_recvlogical. regards, Ajin Cherian

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-06 Thread Amit Kapila
On Fri, Jun 4, 2021 at 2:29 PM Ajin Cherian wrote: > > On Fri, Jun 4, 2021 at 1:06 PM Amit Kapila wrote: > > > I think we can try but not sure if we can get it by then. So, here is > > my suggestion: > > a. remove the change in CreateReplicationSlotCmd > > b. prepare the patches for protocol

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-04 Thread Jeff Davis
On Fri, 2021-06-04 at 08:36 +0530, Amit Kapila wrote: > I think we can try but not sure if we can get it by then. So, here is > my suggestion: > a. remove the change in CreateReplicationSlotCmd > b. prepare the patches for protocol change and pg_recvlogical. This > will anyway include the change

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-04 Thread Ajin Cherian
On Fri, Jun 4, 2021 at 1:06 PM Amit Kapila wrote: > I think we can try but not sure if we can get it by then. So, here is > my suggestion: > a. remove the change in CreateReplicationSlotCmd > b. prepare the patches for protocol change and pg_recvlogical. This > will anyway include the change we

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-03 Thread Amit Kapila
On Thu, Jun 3, 2021 at 10:08 PM Jeff Davis wrote: > > On Thu, 2021-06-03 at 09:29 +0530, Amit Kapila wrote: > > The idea is to support two_phase via protocol with a subscriber-side > > work where we can test it as well. The code to support it via > > replication protocol is present in the first

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-03 Thread Jeff Davis
On Thu, 2021-06-03 at 09:29 +0530, Amit Kapila wrote: > The idea is to support two_phase via protocol with a subscriber-side > work where we can test it as well. The code to support it via > replication protocol is present in the first patch of subscriber-side > work [1] which uses that code as

Re: Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-02 Thread Amit Kapila
On Thu, Jun 3, 2021 at 4:48 AM Jeff Davis wrote: > > Commit 19890a064 changed pg_create_logical_replication_slot() to allow > decoding of two-phase transactions, but did not extend the > CREATE_REPLICATION_SLOT command to support it. Strangely, it does > extend the CreateReplicationSlotCmd struct

Decoding of two-phase xacts missing from CREATE_REPLICATION_SLOT command

2021-06-02 Thread Jeff Davis
Commit 19890a064 changed pg_create_logical_replication_slot() to allow decoding of two-phase transactions, but did not extend the CREATE_REPLICATION_SLOT command to support it. Strangely, it does extend the CreateReplicationSlotCmd struct to add a "two_phase" field, but doesn't set it anywhere.