Re: Fix slot synchronization with two_phase decoding enabled

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix slot synchronization with two_phase decoding enabled
Date: 2025-05-06 05:21:41
Message-ID: CAD21AoAc3QXW58JvHy883T7wyCQMBvEej6o7kYoJgL9f5CPGRQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, May 5, 2025 at 2:59 AM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Fri, May 2, 2025 at 3:05 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Fri, May 2, 2025 at 12:57 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> > >
> > >
> > > Please find the v11 patch addressing the above points and all other
> > > comments. I have also optimized the test by reducing the number of
> > > subscriptions and slots.
> > >
> >
> > Thanks for the patch. Few comments:
> >
> > 1)
> >
> > pg_upgrade/t/003_logical_slots.pl:
> >
> > - "SELECT slot_name, two_phase, failover FROM pg_replication_slots");
> > -is($result, qq(regress_sub|t|t), 'check the slot exists on new cluster');
> > + "SELECT slot_name, two_phase FROM pg_replication_slots");
> > +is($result, qq(regress_sub|t), 'check the slot exists on new cluster');
> >
> > With this change, the failover property test during upgrade is
> > removed. Shall we add it again either using ALTER SUB to enable
> > failover and two_phase together or a new subscription with failover
> > alone?
> >
>
> If ALTER SUB is used to set failover for the existing subscription,
> then pg_upgrade will fail while trying to create a slot with both
> failover and two_phase on the upgraded node $newpub.
> So, I've implemented the other suggestion by adding a separate
> pub-subs to verify the failover property.
>
> > 2)
> > + The default is false. This option cannot be set together with
> > + <literal>failover</literal> when creating a logical replication slot.
> > + However, once the slot is created with <literal>two_phase =
> > true</literal>,
> > + <literal>failover</literal> can be set to true after the slot has
> > + consumed all transactions up to the point where two-phase decoding
> > + was enabled.
> >
> >
> > Suggestion: all transactions --> all the changes.
> >
>
> Done.
> ~~~
>
> Please find the v12 patch with above suggested changes.

Thank you for updating the patch! I'll review it and share comments.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nisha Moond 2025-05-06 05:41:00 Re: pg_createsubscriber: Fix incorrect handling of cleanup flags
Previous Message Daniil Davydov 2025-05-06 05:16:06 Re: POC: Parallel processing of indexes in autovacuum