From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Ajin Cherian <itsajin(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: repeated decoding of prepared transactions |
Date: | 2021-03-01 05:00:01 |
Message-ID: | CALDaNm3z5=ZNMjYNfWkGi3_JZr9DKckKi0NHNs=k1t3qRj6KwQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Mar 1, 2021 at 7:23 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> On Sat, Feb 27, 2021 at 11:06 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> > Few comments on 0002 patch:
> > =========================
> > 1.
> > +
> > + /*
> > + * Disable two-phase here, it will be set in the core if it was
> > + * enabled whole creating the slot.
> > + */
> > + ctx->twophase = false;
> >
> > Typo, /whole/while. I think we don't need to initialize this variable
> > here at all.
> >
> > 2.
> > + /* If twophase is set on the slot at create time, then
> > + * make sure the field in the context is also updated
> > + */
> > + if (MyReplicationSlot->data.twophase)
> > + {
> > + ctx->twophase = true;
> > + }
> > +
> >
> > For multi-line comments, the first line of comment should be empty.
> > Also, I think this is not the right place because the WALSender path
> > needs to set it separately. I guess you can set it in
> > CreateInitDecodingContext/CreateDecodingContext by doing something
> > like
> >
> > ctx->twophase &= MyReplicationSlot->data.twophase
>
> Updated accordingly.
>
> >
> > 3. I think we can support this option at the protocol level in a
> > separate patch where we need to allow it via replication commands (say
> > when we support it in CreateSubscription). Right now, there is nothing
> > to test all the code you have added in repl_gram.y.
> >
>
> Removed that.
>
>
> > 4. I think we can expose this new option via pg_replication_slots.
> >
>
> Done. Added,
>
v7-0002-Add-option-to-enable-two-phase-commits-in-pg_crea.patch adds
twophase to pg_create_logical_replication_slot, I feel this option
should be documented in src/sgml/func.sgml.
Regards,
Vignesh
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2021-03-01 05:02:23 | Re: [HACKERS] Custom compression methods |
Previous Message | Michael Paquier | 2021-03-01 04:57:12 | Re: Different compression methods for FPI |