Re: pg_upgrade allows itself to be run twice

On 01.11.22 14:07, Justin Pryzby wrote:
> On Tue, Nov 01, 2022 at 01:54:35PM +0100, Peter Eisentraut wrote:
>> On 07.07.22 08:22, Justin Pryzby wrote:
>>>> This one comes from NextOID in the control data file after a fresh
>>>> initdb, and GetNewObjectId() would enforce that in a postmaster
>>>> environment to be FirstNormalObjectId when assigning the first user
>>>> OID.  Would you imply an extra step at the end of initdb to update the
>>>> control data file of the new cluster to reflect FirstNormalObjectId?
>>> I added a call to reset xlog, similar to what's in pg_upgrade.
>>> Unfortunately, I don't see an easy way to silence it.
>>
>> I think it would be better to update the control file directly instead of
>> going through pg_resetwal.  (See src/include/common/controldata_utils.h for
>> the required functions.)
>>
>> However, I don't know whether we need to add special provisions that guard
>> against people using postgres --single in complicated ways.  Many consider
>> the single-user mode deprecated outside of initdb use.
> 
> Thanks for looking.

I think the above is a "returned with feedback" at this point.

> One other thing I noticed (by accident!) is that pg_upgrade doesn't
> prevent itself from trying to upgrade a cluster on top of itself:
> 
> | $ /usr/pgsql-15/bin/initdb -D pg15.dat -N
> | $ /usr/pgsql-15/bin/pg_upgrade -D pg15.dat -d pg15.dat -b /usr/pgsql-15/bin
> | Performing Upgrade
> | ------------------
> | Analyzing all rows in the new cluster                       ok
> | Freezing all rows in the new cluster                        ok
> | Deleting files from new pg_xact                             ok
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> | Copying old pg_xact to new server
> | *failure*
> |
> | Consult the last few lines of "pg15.dat/pg_upgrade_output.d/20221101T055916.486/log/pg_upgrade_utility.log" for
>>>
> | command: cp -Rf "pg15.dat/pg_xact" "pg15.dat/pg_xact" >>
"pg15.dat/pg_upgrade_output.d/20221101T055916.486/log/pg_upgrade_utility.log"2>&1
 
> | cp: cannot stat 'pg15.dat/pg_xact': No such file or directory
> 
> This may be of little concern since it's upgrading a version to itself, which
> only applies to developers.

I think this would be worth addressing nonetheless, for robustness.  For 
comparison, "cp" and "mv" will error if you give source and destination 
that are the same file.