MIME-Version: 1.0
References: 
 <CAFPTHDa5aP8ixTZBygGAe48E1N=DvuPzcXikw7bNKixLMN3pVg@mail.gmail.com>
 <OS9PR01MB12149E188221BED862E4BE9E0F5342@OS9PR01MB12149.jpnprd01.prod.outlook.com>
 <CALDaNm2-uwpbJ8fnrssp+hORvOutsqRoZAsa05xVVzXe5Bt3bw@mail.gmail.com>
 <CAFPTHDaNLRqPAda1RUDVfSDH7eLTaONv0UEmc9H5sdMW1Li2Bg@mail.gmail.com>
 <CAN4CZFOb3urMsLPsEyVrYR7-7yA+BC5kDgQQd0nAQ8xj2zyRcA@mail.gmail.com>
 <CAFPTHDYqpuZ6o2-HuCJDYqJ7GY3+zV+Xo-gT7PAgi4Bkz+oTxw@mail.gmail.com>
 <CAN4CZFN4oLdxLwUXHUPV-5mFmK+4dcnppP00fV3i4qmMYCAkGA@mail.gmail.com>
 <CAFPTHDbBiTfjYkQwYTNA2a4LRp8Sp7_zB59fmV0R7977ztxgmg@mail.gmail.com>
 <CAFPTHDaftSwzGTGbFEw8rwDBsub0XqcDm1wxQcquj-Y3PC2qrg@mail.gmail.com>
 <CANhcyEVercsNbgC5CTkMNzh_w_Jv1uK7RQg9vEvAeNQhoBHCKQ@mail.gmail.com>
 <CAJpy0uAXynzWr4w_9GkJC3i=LL9WsRCZYVWAV0PgKmgs8riWzg@mail.gmail.com>
 <CAFPTHDavPyxsWjK0cRO3yOaP4u8FGYrOJuXJB-4wzneAY3H3Ug@mail.gmail.com>
 <CANhcyEVFSutXXdt_0XMCD37NE7Yd7ROdDVEE06D8YVyPNEBFHg@mail.gmail.com>
In-Reply-To: 
 <CANhcyEVFSutXXdt_0XMCD37NE7Yd7ROdDVEE06D8YVyPNEBFHg@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Thu, 11 Jun 2026 12:08:13 +0530
Message-ID: 
 <CAJpy0uD-yDM_ViZm5O+ok3HHMGMnqtdW=oOsuU7SHm9QpNx01w@mail.gmail.com>
Subject: Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
To: Shlok Kyal <shlok.kyal.oss@gmail.com>,
 Amit Kapila <amitkapila16@gmail.com>,
	Dilip Kumar <dilipbalaut@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>,
 Zsolt Parragi <zsolt.parragi@percona.com>,
	vignesh C <vignesh21@gmail.com>,
 "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJpy0uD-yDM_ViZm5O%2Bok3HHMGMnqtdW%3DoOsuU7SHm9QpNx01w%40mail.gmail.com>
Precedence: bulk

On Mon, Jun 8, 2026 at 10:36=E2=80=AFPM Shlok Kyal <shlok.kyal.oss@gmail.co=
m> wrote:
>
>
> 1. I want to clarify, what is the expected behaviour when we upgrade
> the postgres instance from version <=3D 16 to "HEAD + patch"?
> -   ReplicationOriginNameForLogicalRep(subid, InvalidOid, originname,
> sizeof(originname));
> -   replorigin_create(originname);
> +   if (!IsBinaryUpgrade)
> +   {
> +       ReplicationOriginNameForLogicalRep(subid, InvalidOid,
> originname, sizeof(originname));
> +       replorigin_create(originname);
> +   }
> and
> +           /* Dump replication origins */
> +           if (server_version >=3D 170000 && binary_upgrade &&
> archDumpFormat =3D=3D archNull)
> +               dumpReplicationOrigins(conn);
>
> When the postgres instance with subscription is updated from PG 16 (or
> less) to HEAD, the replication origin is created as we do not have "if
> (!IsBinaryUpgrade)" check in HEAD,
> whereas when the similar instance is upgraded to "HEAD + patch", no
> replication origin is present after the upgrade.
> Is this difference in the behaviour between HEAD and "HEAD + patch" expec=
ted?
>

No, this difference is not expected.

The issue is that in patch 002 we intentionally avoid
replication-origin creation during CreateSubscription() by adding the
'!IsBinaryUpgrade' check. The idea is that origin creation should
instead happen through
'binary_upgrade_create_conflict_detection_slot()', where we can also
preserve the
original origin ID.

Origin-ID preservation is currently implemented only for upgrades from
PG17 and later. (see the check in pg_dumpall.c in 002). As a result,
when upgrading from PG16, origin creation is skipped in
CreateSubscription(), but the origin is also not created through
binary_upgrade_create_conflict_detection_slot() because of the PG17
version check. Therefore, no replication origin exists after the
upgrade in this particular case.

We do not have access to the source-node version during the restore
stage (i.e., when CreateSubscription() is executed), so we cannot
perform the version check directly there. Therefore, any information
needed to make this decision must be preserved during the dump stage.

Ajin, Kuroda-san, and I discussed several possible approaches to address th=
is:

1)
Add a new subscription option, for example 'skip_origin_creation'.
When set to true, CREATE SUBSCRIPTION would skip replication-origin
creation. During binary upgrade, pg_dump would emit
skip_origin_creation =3D true for source clusters running PG17 or later,
and false otherwise.

However, this approach requires introducing a new user-visible CREATE
SUBSCRIPTION option.

2)
Another idea could be to have a
'binary_upgrade_skip_subs_origin_creation' variable. We populate it
during dumpSubscription if 'source_version >=3D PG17' and
'IsBinaryUpgrade' is true. So this will be dumped

SELECT pg_catalog.binary_upgrade_set_skip_subs_origin_creation(true);
CREATE SUBSCRIPTION ...
SELECT pg_catalog.binary_upgrade_set_skip_subs_origin_creation(false);

First call will set 'binary_upgrade_skip_subs_origin_creation' which
will be used by Create Subscription to decide to skip origin. This is
similar to how dumpACl decides whether to record priveleges into
'pg_init_privs' for objectes created by extensions. See
binary_upgrade_set_record_init_privs() which sets
binary_upgrade_record_init_privs which is then used by
recordExtensionInitPriv().

3)
Create origins and preserve origin IDs regardless of source version

The current patch preserves origin IDs for upgrades from PG17 and later.

One reason for choosing PG17 as the starting point for preserving
origin-ids is that complete logical-replication setup migration was
introduced in PG17. Prior to PG17, subscription state
(pg_subscription_rel) and replication slots were not preserved during
upgrade, so limiting origin-ID preservation to PG17 and later aligns
with the broader logical-replication migration support.

OTOH, replication-origin migration itself is supported for PG16
upgrades. For PG16- upgrades (without this patch), subscriptions
itself can be migrated, and corresponding origins can be created at
that time. Therefore, even on versions prior to Pg17, it may also be
reasonable to consider preserving origin IDs independently of the rest
of the logical-replication configuration migration, and not tie it to
the PG17 boundary.

If we choose that approach, check_new_cluster_replication_origins()
would likely need to run for upgrades from PG16 (and prior) as well.
Subscription-ids also need to be preserved for the same versions. For
reference, check_new_cluster_replication_slots() and
check_new_cluster_subscription_configuration() currently run only for
PG17 and later.
~~

Given the need for a new user-visible parameter, I would rule out
option 1. Between options 2 and 3, I prefer option 2, but I would be
interested in hearing others' opinions. Any other ideas are also
welcome.

thanks
Shveta