Re: A recent message added to pg_upgade

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, smithpb2250(at)gmail(dot)com, bharath(dot)rupireddyforpostgres(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: A recent message added to pg_upgade
Date: 2025-07-10 12:04:28
Message-ID: CAFiTN-sFsgyCnxsr3oWu0ei=uEue9HTFnsfqcqF5_kqFqLCH0g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 10, 2025 at 2:35 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Jul 10, 2025 at 2:23 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Few comments:
> > 1) With the current approach invalidation will not happen for logical
> > replication slots during upgrade operation, I felt we could retain
> > this assertion just in case in the future it gets called from
> > elsewhere, do you feel this assertion should be removed in the new
> > approach too?
> > - /*
> > - * The logical replication slots shouldn't be invalidated as GUC
> > - * max_slot_wal_keep_size is set to -1 and
> > - * idle_replication_slot_timeout is set to 0 during the binary
> > - * upgrade. See check_old_cluster_for_valid_slots()
> > where we ensure
> > - * that no invalidated before the upgrade.
> > - */
> > - Assert(!(*invalidated && SlotIsLogical(s) && IsBinaryUpgrade));
> >
>
> I don't think we need this assertion. This is a static function, and
> we skipped calling this function in its caller, so it doesn't make
> sense to have this assertion.

I agree.

> > 2) Documentation
> > 2.a) Currently slot invalidation will not happen during upgrade
> > because of idle_replication_slot_timeout, do you feel we should add a
> > note in documentation or is it ok not to mention?
> > 2.b) Currently WAL removal will not happen during upgrade because of
> > max_slot_wal_keep_size, should we add a note about this.
> >
>
> We skip or do a few special things in other parts of the code during
> BinaryUpgrade, but don't mention those, so don't think we need to
> mention this one as well.

Make sense

--
Regards,
Dilip Kumar
Google

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Burd, Greg 2025-07-10 12:13:43 Re: Adding basic NUMA awareness
Previous Message Nisha Moond 2025-07-10 11:47:17 Re: Logical Replication of sequences