On Tue, Dec 23, 2025 at 12:51 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Dec 23, 2025 at 08:21:39AM +0900, Michael Paquier wrote:
> > Creating the origin at the end of the same transaction that sets the
> > state to SUBREL_STATE_DATASYNC seems sensible here. I'll spend a
> > couple of extra hours playing with all that across all the branches,
> > see if I can wrap it. This includes some more error injection to
> > cross-check the state of all these transactions with the states in
> > the catalogs while we drop the subscription.
>
> While looking at the state of the code across the six branches where
> we need to fix this, there were two points that have been slightly
> sticky on my mind:
> 1) check_old_cluster_subscription_state() in pg_upgrade's check.c,
> where we have a set of comments dealing with the reasons why only the
> initial and ready states are allowed for the transfers of the relation
> data. The patch only makes the origin visible in the catalogs for one
> extra state, DATASYNC now, meaning that we have nothing to care about.
> I was wondering about the comment of DATASYNC being slightly incorrect
> now because it only mentions a replication slot. Do you think that we
> should adjust that as well to mention the case of origins, knowing
> that their names are also based on subscription OIDs whose value
> change across upgrades? That would not apply for relation IDs as
> these are fixed, but this feels a bit inexact now for the branches
> where this code applies.
>
You are right. How about attached to make it match with the current code?
--
With Regards,
Amit Kapila.