Re: Separate GUC for replication origins

On Wed, Mar 5, 2025 at 12:42 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Mar 4, 2025 at 10:42 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Mar 5, 2025 at 6:24 AM Euler Taveira <euler@eulerto.com> wrote:
> > >
> > > On Sat, Mar 1, 2025, at 10:08 AM, Amit Kapila wrote:
> > >
> > > On Thu, Feb 13, 2025 at 6:48 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > Thank you for updating the patch! The patch looks mostly good to me.
> > > >
> > >
> > > + /*
> > > + * Prior to PostgreSQL 18, max_replication_slots was used to set the
> > > + * number of replication origins. For backward compatibility, -1 indicates
> > > + * to use the fallback value (max_replication_slots).
> > > + */
> > > + if (max_replication_origin_sessions == -1)
> > >
> > > Shouldn't we let users use max_replication_origin_sessions for
> > > subscribers? Maintaining this mapping for a long time can create
> > > confusion such that some users will keep using max_replication_slots
> > > for origins, and others will start using
> > > max_replication_origin_sessions. Such a mapping would be useful if we
> > > were doing something like this in back-branches, but doing it in a
> > > major version appears to be more of a maintenance burden.
> > >
> > >
> > > It was my initial proposal. Of course, it breaks compatibility but it
> > > forces the users to adopt this new GUC. It will be a smooth adoption
> > > because if we use the same value as max_replication_slots it means
> > > most of the scenarios will be covered (10 is a good start point for the
> > > majority of replication scenarios in my experience).
> > >
> > > However, Peter E suggested that it would be a good idea to have a
> > > backward compatibility so I added it.
> > >
> > > We need to decide which option we want:
> > >
> > > 1. no backward compatibility and max_replication_origin_sessions = 10 as
> > >   default value. It might break scenarios that have the number of
> > >   subscriptions greater than the default value.
> > >
> >
> > To avoid breakage, can we add a check during the upgrade to ensure
> > that users have set max_replication_origin_sessions appropriately? See
> > the current check in function
> > check_new_cluster_subscription_configuration(). It uses
> > max_replication_slots, we can change it to use
> > max_replication_origin_sessions for versions greater than or equal to
> > 18. Will that address this concern?
>
> I think that the patch already replaced max_replication_slots with
> max_replication_origin_sessions in that function.
>

Then, that should be sufficient to avoid upgrade related risks.

--
With Regards,
Amit Kapila.