Dear Chao,
> I forgot to mention one small suggestion.
>
> Since the internal log file path includes a timestamped directory, it is a bit
> inconvenient to figure out the full path of the file after the command finishes. I
> think it would be helpful to print the full path of the internal log file at the end,
> something like:
> ```
> if (internal_log_file_fp != NULL)
> {
> if (fclose(internal_log_file_fp) != 0)
> report_createsub_fatal("could not
> close %s/%s.log: %m", logdir, INTERNAL_LOG_FILE_NAME);
> internal_log_file_fp = NULL;
> printf("check the internal log file at \"%s/%s.log\"", logdir,
> INTERNAL_LOG_FILE_NAME);
> }
> ```
I don't think it's needed, because pg_upgrade does not print such points.
> Note that I used printf() here, because based on my test, pg_log_info() does not
> work in this place.
It might be becasue you used pg_log_info(). pg_createsubscriber outputs only
warning or error level messages, info-level cannot be printed.
> The other thing I noticed while trying the above code is that
> cleanup_objects_atexit() has this:
> ```
> if (success)
> return;
> ```
>
> That means on the success path, internal_log_file_fp will not be closed. So this
> part would need some adjustment as well. Maybe one way is to jump to the
> log-file cleanup part before returning, for example with a goto, so that
> internal_log_file_fp is always closed.
As the same reason I posted, I think it's not needed.
[1]:
https://www.postgresql.org/message-id/OS9PR01MB12149F1AD9C79A2644753A18AF548A%40OS9PR01MB12149.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
