On Thu, Dec 17, 2020 at 7:02 AM Ajin Cherian <itsajin@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.
