On Fri, Aug 21, 2026 at 10:19 AM Ajin Cherian <[email protected]> wrote:
>
> On Wed, Aug 19, 2026 at 7:52 PM shveta malik <[email protected]> wrote:
> >
> > On Wed, Aug 19, 2026 at 11:13 AM Ajin Cherian <[email protected]> wrote:
> > >
> > >
> > > I have incorporated your comments and your changes in a new patch v16.
> > >
> >
> > Please find a few minor comments:
> >
>
> I've addressed all the comments except the below one.
>
> >
> > 4)
> >
> > +    old subscriber was up to. Migration of the subscription's replication
> > +    origin is supported for all cluster versions.
> >
> > Shall we say:
> > all supported cluster versions
>
> It is stating the obvious saying that it is supported for all
> supported cluster versions, so I didn't change this.
>

Okay. Thanks Ajin.


I have one trivial concern to discuss. We document this:

---
The new cluster must have max_active_replication_origins configured to
a value greater than or equal to the number of tracked replication
origins present in the old cluster (reflected in
pg_replication_origin_status). Furthermore, the new cluster must not
contain any pre-existing replication origins.
---

But we check for pre-existing origins in the new cluster only if the
old cluster has any origins. So, the second check is effectively
conditional. This is probably not harmful; if the old cluster has no
origins, there is nothing to migrate, so we don't really need to worry
about the origin count.
count on the new cluster. But the doc and code are not quite in sync:

check_new_cluster_replication_origins:
+ if (old_cluster.nrepl_origins == 0)
  return;

I noticed a similar inconsistency with replication slots, we mention
this in doc [1]: 'The new cluster must not have any permanent logical
slots',

But the code also conditionally skips the check:
         /*
         * Quick return if there are no slots to be migrated and no
subscriptions
         * have the retain_dead_tuples option enabled.
         */
        if (nslots_on_old == 0 && !old_cluster.sub_retain_dead_tuples)
                return;

So, is the current behavior okay, or should we:

a) Check for pre-existing origins on the new cluster regardless of the
old cluster's origin count?
b) Or update the doc to make it consistent with code, at-least for our
origin case?
c) Leave things as they are?

[1]: https://www.postgresql.org/docs/current/logical-replication-upgrade.html


thanks
Shveta


Reply via email to