On Fri, Jun 03, 2022 at 10:32:27PM -0500, Justin Pryzby wrote:
> On Sat, Jun 04, 2022 at 12:13:19PM +0900, Michael Paquier wrote:
>> I am not so sure. My first reaction was actually to bypass the
>> creation of the directories on EEXIST.
>
> But that breaks the original motive behind the patch I wrote - logfiles are
> appended to, even if they're full of errors that were fixed before re-running
> pg_upgrade.
Yep.
>> But, isn't the problem different and actually older here? In the set of
>> commands given by Tushar, he uses the --check option without --retain, but
>> the logs are kept around after the execution of the command. It seems to me
>> that there is an argument for also removing the logs if the caller of the
>> command does not want to retain them.
>
> You're right that --check is a bit inconsistent, but I don't think we could
> backpatch any change to fix it. It wouldn't matter much anyway, since the
> usual workflow would be "pg_upgrade --check && pg_upgrade". In which case the
> logs would end up being removed anyway.
Exactly, the inconsistency in the log handling is annoying, and
cleaning up the logs when --retain is not used makes sense to me when
the --check command succeeds, but we should keep them if the --check
fails. I don't see an argument in backpatching that either.
> Hmm .. maybe what you mean is that the *tap test* should first run with
> --check?
Sorry for the confusion. I meant to add an extra command in the TAP
test itself.
I would suggest the attached patch then, to add a --check command in
the test suite, with a change to clean up the logs when --check is
used without --retain.
--
Michael