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.
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
