Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

On Tue, Jul 21, 2026 at 9:04 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>
> 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?
>

The reason for this difference in behaviour is that on head,
replorigin_advance is called unconditionally even if remote_lsn = 0/0,
which is even if the origin hasn't replayed anything (e.g.,
subscription hasn't started replicating, or table sync hasn't
progressed past the initial state). So, the origin has an entry in
pg_replication_status. (Should it?)
With the patch, although the replication origin is created, it is
advanced at upgrade time only if it has a meaningful remote_lsn and
otherwise it will eventually get updated when the apply worker starts
up. But, in the test scenario, the apply worker never gets a chance to
run because max_logical_replication_workers is set to 0. I think this
behaviour is fine. But if maintaining consistency with previous
versions is important, I also understand that.
The particular code that causes this difference is:

+ if (remote_lsn != InvalidXLogRecPtr)
+ {
+ /*
+ * The remote_lsn is expected to be valid only during binary upgrade when
+ * preserving an existing replication origin. For the normal origin
+ * creation flow, it should always be InvalidXLogRecPtr.
+ */
+ Assert(IsBinaryUpgrade);
+
+ replorigin_advance(roident, remote_lsn, InvalidXLogRecPtr,
+ false /* backward */,
+ false /* WAL log */);
+ }

The code does not differentiate between a remote_lsn of 0/0 and the
caller passing InvalidXLogRecPtr. I could revert to older behaviour by
changing the if condition to if (IsBinaryUpgrade).

Let me know what people think? Keep the existing behaviour or is the
new behaviour fine?

regards,
Ajin Cherian
Fujitsu Australia