On Thu, Dec 17, 2020 at 7:02 AM Ajin Cherian <itsa...@gmail.com> wrote: > > > v31-0009-Support-2PC-txn-Subscription-option > > 1. > > --- a/src/include/catalog/catversion.h > > +++ b/src/include/catalog/catversion.h > > @@ -53,6 +53,6 @@ > > */ > > > > /* yyyymmddN */ > > -#define CATALOG_VERSION_NO 202011251 > > +#define CATALOG_VERSION_NO 202011271 > > > > No need to change catversion as this gets changed frequently and that > > leads to conflict in the patch. We can change it either in the final > > version or normally committers take care of this. If you want to > > remember it, maybe adding a line for it in the commit message should > > be okay. For now, I have removed this from the patch. > > > > > > -- > > With Regards, > > Amit Kapila. > > I have reviewed the changes, did not have any new comments. > While testing, I found an issue in this patch. During initialisation, > the pg_output is not initialised fully and the subscription parameters > are not all read. As a result, ctx->twophase could be > set to true , even if the subscription does not specify so. For this, > we need to make the following change in pgoutput.c: > pgoutput_startup(), similar to how streaming is handled. > > /* > * This is replication start and not slot initialization. > * > * Parse and validate options passed by the client. > */ > if (!is_init) > { > : > : > } > else > { > /* Disable the streaming during the slot initialization mode. */ > ctx->streaming = false; > + ctx->twophase = false > } >
makes sense. I can take care of this in the next version where I am planning to address Sawada-San's comments and few other clean up work. -- With Regards, Amit Kapila.