On Thu, Jun 10, 2021 at 4:13 AM Jeff Davis <pg...@j-davis.com> 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 the worker.c why we can't > > enable it during CREATE_REPLICATION_SLOT. > > I'll have to catch up on the thread to digest that reasoning and how it > applies to decoding vs. replication. But there don't seem to be changes > to START_REPLICATION for twophase, so I don't quite follow your point. >
I think it is because we pass there as an option as I have suggested doing in the case of CREATE_REPLICATION_SLOT. > Are you saying that we should not be able to create slots with twophase > at all until the rest of the changes go in? > No, the slots will be created but two_phase option will be enabled only after the initial tablesync is complete. > > Now, if we want to do > > protocol changes, I wonder why only do some changes and leave the > > rest > > for the next version? > > I started this thread because it's possible to create a slot a certain > way using the SQL function create_logical_replication_slot(), but it's > impossible over the replication protocol. That seems inconsistent to > me. > Right, I understand that but on the protocol side, there are few more things to be considered to allow subscribers to enable two_phase. However, maybe, for now, we can do it just for create_replication_slot and the start_replication stuff required for subscribers can be done later. I was not completely sure if that is a good idea. -- With Regards, Amit Kapila.
