Re: pg_upgrade should truncate/remove its logs before running

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>, Daniel Gustafsson <daniel@yesql.se>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Andrew Dunstan <andrew@dunslane.net>, Bruce Momjian <bruce@momjian.us>, pgsql-hackers@lists.postgresql.org
Date: 2022-01-08T18:48:57Z
Lists: pgsql-hackers

Attachments

The cfbot was failing under windows:
| [22:07:02.159] could not create directory "pg_upgrade_output.d": File exists

It's because parseCommandLine() was called before get_restricted_token(), which
re-executes the process, and runs parseCommandLine again.

parseCommandLine already does stuff like opening logfiles, so that's where my
mkdir() is.  It fails when re-run, since the re-exec doesn't call the cleanup()
path.

I fixed it by calling get_restricted_token() before parseCommandLine().
There's precedent for that in pg_regress (but the 3 other callers do it
differently).

It seems more ideal to always call get_restricted_token sooner than later, but
for now I only changed pg_upgrade.  It's probably also better if
parseCommandLine() only parses the commandline, but for now I added on to the
logfile stuff that's already there.

BTW the CI integration is pretty swell.  I added a few lines of debugging code
to figure out what was happening here.  check world on 4 OSes is faster than
check world run locally.  I rearranged cirrus.yaml to make windows run its
upgrade check first to save a few minutes.

Maybe the commandline argument should be callled something other than "logdir"
since it also outputs dumps there.  But the dumps are more or less not
user-facing.  But -d and -o are already used.  Maybe it shouldn't be
configurable at all?

-- 
Justin

Commits

  1. pg_upgrade: Move all the files generated internally to a subdirectory