On Sun, Jul 26, 2026 at 9:03 PM Rui Zhao <[email protected]> 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
