> On 5 Jun 2022, at 11:19, Michael Paquier <mich...@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/
