Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

>
> Hi Ajin,
>
> Thanks for v8. A couple of comments:
>
> 1) The PG17 gating seems inconsistent between the dump and the check now.
> dumpReplicationOrigins() is called unconditionally:
>
>     if (binary_upgrade && archDumpFormat == archNull)
>         dumpReplicationOrigins(conn);
>
> but get_subscription_info(), which sets nrepl_origins, is still only
> called for old clusters >= 17 in check_new_cluster():
>
>     if (GET_MAJOR_VERSION(old_cluster.major_version) >= 1700)
>         ...
>         get_subscription_info(&old_cluster);
>
> and check_new_cluster_replication_origins() returns early on it:
>
>     if (old_cluster.nrepl_origins == 0)
>         return;
>
> For a < 17 old cluster get_subscription_info() is never called, so
> nrepl_origins stays 0 and this returns early. So when such a cluster has a
> (user-created) origin, the origin still gets dumped and recreated, while
> the "new cluster must have no origins" and max_active_replication_origins
> checks are skipped, and the upgrade fails during restore instead of at the
> check step. nrepl_origins is just a count(*) on pg_replication_origin, so
> moving its collection out of the >= 1700 block should be enough.
>
> 2) 004_subscription.pl only checks subscription origins (pg_<oid>). Since
> the patch migrates all origins, should we also create a user origin
> (pg_replication_origin_create()) and verify its roident/remote_lsn after
> the upgrade? That would also cover the case in (1).
>
> Thanks,
> Rui
>