Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: Ajin Cherian <itsajin(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
Date: 2026-07-21 11:04:41
Message-ID: CANhcyEX8RFETPqYjXwBeGhQEHVdrdaoRyWssHP7DAEjXRs5=Ug@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 21 Jul 2026 at 15:46, Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> I've addressed all the above comments.
>
> There was also a CFbot failure on linux machine as below:
>
> # Failed test 'run of pg_upgrade where the new cluster has
> insufficient max_active_replication_origins status (got 0 vs expected
> 1)'
> [02:43:22] t/001_basic.pl ................... ok 180 ms ( 0.00
> usr 0.00 sys + 0.11 cusr 0.04 csys = 0.15 CPU)
> # at t/004_subscription.pl line 60.
>
> I don't see the same error on my machine, but I think the problem is
> because the node is stopped too soon after the subscription is
> created. So I've added logic to wait until the replication origin
> becomes active.
>
> +# Wait until the apply worker sets up the replication origin, making it
> +# active. Otherwise, stopping the old cluster too early could leave the
> +# origin inactive, causing the check below to see no active replication
> +# origins and fail.
> +$old_sub->poll_query_until('postgres',
> + "SELECT count(*) > 0 FROM pg_replication_origin_status")
> + or die "Timed out while waiting for the replication origin to become active";
> +
> $old_sub->stop;
>
Thanks for the updated patch.
I was reviewing v12. Please find the comments for the patch (these
comments are also applicable for v13):

1. I think a blank line after the pg_fatal, will make the code more readable:
+ if (PQntuples(res) != 1)
+ pg_fatal("could not get the number of active replication origins");
+ cluster->nrepl_origins_active = atoi(PQgetvalue(res, 0, 0));
+ PQclear(res);

2. I was testing the patch and found a difference of behaviour between
HEAD and with Patch.
Suppose we have two nodes, publisher and subscriber, and the
subscriber has two subscriptions. The replication origin states for
the respective subscriptions are:
local_id | external_id | remote_lsn | local_lsn
----------+-------------+------------+------------
1 | pg_16393 | 0/00000000 | 0/0174F518
2 | pg_16394 | 0/017516C0 | 0/0174FDA0
Now, we want to upgrade the subscriber node. On the
upgraded_subscriber node, max_logical_replication_workers is set to 0.

With this patch, the replication origin states after the upgrade are:
local_id | external_id | remote_lsn | local_lsn
----------+-------------+------------+------------
2 | pg_16394 | 0/017516C0 | 0/00000000
(1 row)

Whereas on HEAD, the replication origin states after the upgrade are:
local_id | external_id | remote_lsn | local_lsn
----------+-------------+------------+------------
1 | pg_16400 | 0/00000000 | 0/00000000
2 | pg_16401 | 0/017516C0 | 0/00000000

With this patch, I observed that replication origins whose remote_lsn
is '0/00000000' are not present on the upgraded node when
max_logical_replication_workers is set to 0.
Is this behaviour expected?

Thanks,
Shlok Kyal

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nazir Bilal Yavuz 2026-07-21 11:26:35 Re: ci: namespace ccache by PostgreSQL major version
Previous Message Ayush Tiwari 2026-07-21 10:59:10 Re: ri_Fast* crash w/ nullable UNIQUE constraint