Re: pg_upgrade should truncate/remove its logs before running

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pg_upgrade should truncate/remove its logs before running
Date: 2021-12-20 11:21:51
Message-ID: YcBnT2awK89o6P5I@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 17, 2021 at 11:21:13AM -0600, Justin Pryzby wrote:
> I put this together in the simplest way, prefixing all the filenames with the
> configured path..

Well, why not.

> Another options is to chdir() into the given path. But, pg_upgrade takes (and
> requires) a bunch of other paths, like -d -D -b -B, and those are traditionally
> interpretted relative to CWD. I could getcwd() and prefix all the -[dDbB] with
> that, but prefixing a handful of binary/data paths is hardly better than
> prefixing a handful of dump/logfile paths. I suppose that openat() isn't
> portable. I don't think this it's worth prohibiting relative paths, so I can't
> think of any less-naive way to do this.

If we add a new file, .gitignore would find about it quickly and
inform about a not-so-clean tree. I would tend to prefer your
approach, here. Relative paths can be useful.

> I didn't move the delete-old-cluster.sh, since that's intended to stay around
> even after a successful upgrade, as opposed to the other logs, which are
> typically removed at that point.

Makes sense to me.

+ log_opts.basedir = getenv("PG_UPGRADE_LOGDIR");
+ if (log_opts.basedir != NULL)
+ log_opts.basedir = strdup(log_opts.basedir);
+ else
+ log_opts.basedir = "pg_upgrade_log.d";
Why is this controlled with an environment variable? It seems to me
that an option switch would be much better, no? While tuning things,
we could choose something simpler for the default, like
"pg_upgrade_log". I don't have a good history in naming new things,
though :)

.gitignore should be updated, I guess? Besides, this patch has no
documentation.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-12-20 11:30:02 Use JOIN USING aliases in ruleutils.c
Previous Message Michael Paquier 2021-12-20 11:11:04 Re: Confused comment about drop replica identity index