Re: New standby_slot_names GUC in PG 17

On Wed, Jun 26, 2024 at 6:15 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Wednesday, June 26, 2024 12:49 PM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Hi,
> >
> > On Wed, Jun 26, 2024 at 04:17:45AM +0000, Zhijie Hou (Fujitsu) wrote:
> > > On Wednesday, June 26, 2024 9:40 AM Masahiko Sawada
> > <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Tue, Jun 25, 2024 at 5:32 PM Amit Kapila
> > > > <amit.kapila16@gmail.com>
> > > > wrote:
> > > > >
> > > > > I feel synchronized better indicates the purpose because we ensure
> > > > > such slots are synchronized before we process changes for logical
> > > > > failover slots. We already have a 'failover' option for logical
> > > > > slots which could make things confusing if we add 'failover' where
> > > > > physical slots need to be specified.
> > > >
> > > > Agreed. So +1 for synchronized_stnadby_slots.
> > >
> > > +1.
> > >
> > > Since there is a consensus on this name, I am attaching the patch to
> > > rename the GUC to synchronized_stnadby_slots. I have confirmed that
> > > the regression tests and pgindent passed for the patch.
> > A few comments:
>
> Thanks for the comments!
>
> > 1 ====
> >
> > In the commit message:
> >
> > "
> > The standby_slot_names GUC is intended to allow specification of physical
> >     standby slots that must be synchronized before they are visible to
> >     subscribers
> > "
> >
> > Not sure that wording is correct, if we feel the need to explain the GUC, maybe
> > repeat some wording from bf279ddd1c?
>
> I intentionally copied some words from release note of this GUC which was
> also part of the content in the initial email of this thread. I think it
> would be easy to understand than the original commit msg. But others may
> have different opinion, so I would leave the decision to the committer. (I adjusted
> a bit the word in this version).
>
> >
> > 2 ====
> >
> > Should we rename StandbySlotNamesConfigData too?
> >
> > 3 ====
> >
> > Should we rename SlotExistsInStandbySlotNames too?
> >
> > 4 ====
> >
> > Should we rename validate_standby_slots() too?
> >
>
> Renamed these to the names suggested by Amit.
>
> Attach the v2 patch set which addressed above and removed
> the changes in release-17.sgml according to the comment from Amit.
>

Thank you for updating the patch. The v2 patch looks good to me.

Regards,

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