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