On Wed, Jan 24, 2024 at 2:38 PM Bertrand Drouvot
<bertranddrouvot...@gmail.com> wrote:
>
> Hi,
>
> On Wed, Jan 24, 2024 at 01:51:54PM +0530, Amit Kapila wrote:
> > On Wed, Jan 24, 2024 at 11:24 AM Masahiko Sawada <sawada.m...@gmail.com> 
> > wrote:
> > >
> > > On Wed, Jan 24, 2024 at 2:43 PM Amit Kapila <amit.kapil...@gmail.com> 
> > > wrote:
> > > >
> > > > >
> > > > > +/* GUC variable */
> > > > > +bool       enable_syncslot = false;
> > > > >
> > > > > Is enable_syncslot a really good name? We use "enable" prefix only for
> > > > > planner parameters such as enable_seqscan, and it seems to me that
> > > > > "slot" is not specific. Other candidates are:
> > > > >
> > > > > * synchronize_replication_slots = on|off
> > > > > * synchronize_failover_slots = on|off
> > > > >
> > > >
> > > > I would prefer the second one. Would it be better to just say
> > > > sync_failover_slots?
> > >
> > > Works for me. But if we want to extend this option for non-failover
> > > slots as well in the future, synchronize_replication_slots (or
> > > sync_replication_slots) seems better. We can extend it by having an
> > > enum later. For example, the values can be on, off, or failover etc.
> > >
> >
> > I see your point. Let us see if others have any suggestions on this.
>
> I also see Sawada-San's point and I'd vote for "sync_replication_slots". Then 
> for
> the current feature I think "failover" and "on" should be the values to turn 
> the
> feature on (assuming "on" would mean "all kind of supported slots").

Even if others agree and we change this GUC name to
"sync_replication_slots", I feel we should keep the values as "on" and
"off" currently, where "on" would mean 'sync failover slots' (docs can
state that clearly).  I do not think we should support sync of "all
kinds of supported slots" in the first version. Maybe we can think
about it for future versions.

thanks
Shveta


Reply via email to