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

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: Re: [v15 beta] pg_upgrade failed if earlier executed with -c switch
Date: 2022-06-06 00:38:03
Message-ID: E72AF81D-776B-4DC8-88E0-9650E8EA0D41@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 5 Jun 2022, at 11:19, Michael Paquier <michael(at)paquier(dot)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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-06-06 04:17:52 Re: [v15 beta] pg_upgrade failed if earlier executed with -c switch
Previous Message Matthias van de Meent 2022-06-05 19:51:47 Re: [PATCH] Compression dictionaries for JSONB