Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

On Sun, Jul 26, 2026 at 9:03 PM Rui Zhao <zhaorui126(at)gmail(dot)com> wrote:
>
> Hi Ajin,
>
> One more thing for the remote_lsn = 0/0 question Shlok raised: whichever way it
> is settled, the new max_active_replication_origins check should follow it,
> because right now the two disagree.
>
> The check counts status rows on the old cluster -- get_replication_origin_info()
> fills nrepl_origins_active from
>
> SELECT count(*) FROM pg_catalog.pg_replication_origin_status
>
> and check_new_cluster_replication_origins() compares that against the setting:
>
> if (old_cluster.nrepl_origins_active > max_active_replication_origins)
>
> The rationale for counting tracked origins rather than all of them was that this
> is what decides whether the new server comes up ("Setting it to a lower value
> than the current number of tracked replication origins ... will prevent the
> server from starting"). But restore does not reproduce that set on the new
> cluster. An origin sitting at 0/0 has a status row on the old cluster and is
> counted, while replorigin_create_with_id() skips the advance for it, since
> 0/0 is InvalidXLogRecPtr:
>
> if (remote_lsn != InvalidXLogRecPtr)
> replorigin_advance(...);
>
> so it ends up untracked on the new cluster and counts for nothing there. The
> check is therefore comparing the setting against a larger set than the one the
> new server will actually have to start with.
>
> Two user origins, one advanced and one left at 0/0:
>
> SELECT pg_replication_origin_create('uo_advanced');
> SELECT pg_replication_origin_advance('uo_advanced', '0/12345678');
> SELECT pg_replication_origin_create('uo_zero');
> SELECT pg_replication_origin_advance('uo_zero', '0/0');
> -- 2 status rows
>
> New cluster with max_active_replication_origins = 1:
>
> "max_active_replication_origins" (1) must be greater than or equal to the
> number of active replication origins (2) in the old cluster
>
> -- refused, though 1 is in fact enough. Run it through with a higher setting and
> the new cluster ends up with a single status row (uo_advanced); uo_zero is there
> in pg_replication_origin with its roident preserved, but untracked. Dropping
> uo_zero on the old cluster and re-running with the same setting of 1 passes,
> which is what pins it on the 0/0 origin.
>
> So if the skip stays as it is, the count should follow it -- counting only
> status rows whose remote_lsn is non-zero. If instead it goes back to advancing
> unconditionally, the two line up on their own and the check needs no change.
> Either is fine by me; it's the pair being out of step that seemed worth
> mentioning while that behaviour is still being decided.
>

Okay, I see your point. In that case, I think we should track the
origin with lsn = 0x0 rather than skip it. To me, this seems more
understandable than counting only the status rows whose remote_lsn is
non-zero.

> Minor, unrelated: the doc note was updated to "Commit timestamps are not
> preserved during the upgrade", but the twin comment in pg_upgrade.c still reads
>
> * upgrade. Additionally, commit timestamps and origin data are not
> * preserved during the upgrade. ...
>
> which is no longer true now that this patch preserves origins -- same
> edit as the
> .sgml. (I mentioned this on v10; just flagging it's still there.)
>
> Thanks,
> Rui