Dear Vignesh,
Thank you for reviewing! PSA new version.
>
> Few comments:
> 1) Most of the code in binary_upgrade_set_next_oid is similar to
> SetNextObjectId, but SetNextObjectId has the error handling to report
> an error if an invalid nextOid is specified:
> if (ShmemVariableCache->nextOid > nextOid)
> elog(ERROR, "too late to advance OID counter to %u, it is now %u",
> nextOid, ShmemVariableCache->nextOid);
>
> Is this check been left out from binary_upgrade_set_next_oid function
> intentionally? Have you left this because it could be like a dead
> code. If so, should we have an assert for this here?
Yeah, they were removed intentionally, but I did rethink that they could be
combined. ereport() would be skipped during the upgrade mode. Thought?
Regarding the first ereport(ERROR), it just requires that we are doing initdb.
As for the second ereport(ERROR), it requires that next OID is not rollbacked.
The restriction seems OK during the initialization, but it is not appropriate for
upgrading: wraparound of OID counter might be occurred on old cluster but we try
to restore the counter anyway.
> 2) How about changing issue_warnings_and_set_oid function name to
> issue_warnings_and_set_next_oid?
> void
> -issue_warnings_and_set_wal_level(void)
> +issue_warnings_and_set_oid(void)
> {
Fixed.
> 3) We have removed these comments, is there any change to the rsync
> instructions? If so we could update the comments accordingly.
> - * We unconditionally start/stop the new server because
> pg_resetwal -o set
> - * wal_level to 'minimum'. If the user is upgrading standby
> servers using
> - * the rsync instructions, they will need pg_upgrade to write its final
> - * WAL record showing wal_level as 'replica'.
>
Hmm, I thought comments for rsync seemed outdated so that removed. I still think
this is not needed. Since controlfile->wal_level is not updated to 'minimal'
anymore, the unconditional startup is not required for physical standby.
[1] :
https://www.postgresql.org/docs/devel/pgupgrade.html#:~:text=the%20next%20step.-,Run%20rsync,-When%20using%20link
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
