Re: pg_upgrade allows itself to be run twice

On Thu, Dec 01, 2022 at 10:30:16AM +0100, Peter Eisentraut wrote:
> 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.

To be clear, I don't think it's worth doing anything special just to
avoid creating special OIDs if someone runs postgres --single
immediately after initdb.

However, setting FirstNormalOid in initdb itself (rather than in the
next invocation of postgres, if it isn't in single-user-mode) was the
mechanism by which to avoid the original problem that pg_upgrade can be
run twice, if there are no non-system relations. That still seems
desirable to fix somehow.

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

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

I addressed this in 002.

--
Justin