On Wed, Mar 5, 2025 at 12:42 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Mar 4, 2025 at 10:42 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, Mar 5, 2025 at 6:24 AM Euler Taveira <eu...@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.m...@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.
