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

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