Re: Separate GUC for replication origins

On Thu, Mar 20, 2025 at 8:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Mar 20, 2025 at 10:37 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Mar 19, 2025 at 8:15 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, Mar 19, 2025 at 10:43 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 17, 2025 at 6:05 PM Euler Taveira <euler@eulerto.com> wrote:
> > > > >
> > > > > On Mon, Mar 17, 2025, at 8:44 PM, Masahiko Sawada wrote:
> > > > >
> > > > > I would suggest putting the new max_active_replication_origins after
> > > > > max_parallel_apply_workers_per_subscription as both
> > > > > max_sync_workers_per_subscription and
> > > > > max_parallel_apply_workers_per_subscription are related to
> > > > > max_logical_replication_workers.
> > > > >
> > > > >
> > > > > Good point. Looking at the documentation, the old max_replication_slots
> > > > > parameter was the first one in that section so I decided to use the same order
> > > > > for the postgresql.conf.sample.
> > > >
> > > > Thank you for updating the patch!
> > > >
> > >
> > > *
> > >    <para>
> > >     Logical replication requires several configuration options to be set. Most
> > > -   options are relevant only on one side of the replication. However,
> > > -   <varname>max_replication_slots</varname> is used on both the publisher and
> > > -   the subscriber, but it has a different meaning for each.
> > > +   options are relevant only on one side of the replication.
> > >    </para>
> > >
> > > In this para, after removing the content about max_replication_slots,
> > > the other line: "Most options are relevant only on one side of the
> > > replication." doesn't make sense because there is no other option that
> > > applies to both sides and if there is one then we should mention about
> > > that.
> >
> > Good point. How about the following change?
> >
> >    <para>
> > -   Logical replication requires several configuration options to be set. Most
> > +   Logical replication requires several configuration options to be set. These
> >     options are relevant only on one side of the replication.
> >    </para>
> >
>
> LGTM.

Thank you for reviewing it. I've committed the patch.


Regards,

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