Re: [v15 beta] pg_upgrade failed if earlier executed with -c switch

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Michael Paquier <michael@paquier.xyz>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Tom Lane <tgl@sss.pgh.pa.us>, tushar <tushar.ahuja@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Andrew Dunstan <andrew@dunslane.net>
Date: 2022-06-06T00:38:03Z
Lists: pgsql-hackers
> On 5 Jun 2022, at 11:19, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Sun, Jun 05, 2022 at 09:24:25AM +0900, Michael Paquier wrote:
>> Well, another error that could happen in the early code paths is
>> EACCES on a custom socket directory specified, and we'd still face the
>> same problem on a follow-up restart.  Using a sub-directory structure
>> as Daniel and Tom mention would address all that (if ignoring EEXIST
>> for the BASE_OUTPUTDIR), removing any existing content from the base
>> path when not using --retain.  This comes with the disadvantage of
>> bloating the disk on repeated errors, but this last bit would not
>> really be a huge problem, I guess, as it could be more useful to keep
>> the error information around.
> 
> I have been toying with the idea of a sub-directory named with a
> timestamp (Unix time, like log_line_prefix's %n but this could be
> any format) under pg_upgrade_output.d/ and finished with the
> attached.  

I was thinking more along the lines of %m to make it (more) human readable, but
I'm certainly not wedded to any format.

> The logs are removed from the root path when --check is
> used without --retain, like for a non-check command.

This removes all logs after a command without --retain, even if a previous
command used --retain to keep the logs around.

As a user I would expect the logs from this current invocation to be removed
without --retain, and any other older log entries be kept.  I think we should
remove log_opts.logdir and only remove log_opts.rootdir if it is left empty
after .logdir is removed.

> The logic in charge of cleaning up the logs has been moved to a single
> routine, aka cleanup_logs().

+		cleanup_logs();

Maybe we should register cleanup_logs() as an atexit() handler once we're done
with option processing?

+	snprintf(log_opts.logdir, MAXPGPATH, "%s/%s/%s", log_opts.rootdir,
+			 timebuf, LOG_OUTPUTDIR);

While not introduced by this patch, it does make me uneasy that we create paths
without checking for buffer overflows..

--
Daniel Gustafsson		https://vmware.com/




Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Restructure pg_upgrade output directories for better idempotence