On Mon, 26 Jan 2026 at 07:08, Gyan Sreejith <gyan.sreejith@gmail.com> wrote: > > Thank you, I have made the changes and attached the patch.
Few comments: 1) Adding \n at the end will assert as pg_log_generic_v has the following Assert: Assert(fmt[strlen(fmt) - 1] != '\n');
@@ -1106,7 +1220,7 @@ check_subscriber(const struct LogicalRepInfo *dbinfo) int max_reporigins; int max_wprocs;
- pg_log_info("checking settings on subscriber"); + INFO("checking settings on subscriber\n");
You can run in verbose mode without -l to see this issue
2) There is a chance that directory creation can fail, it should be checked and error should be thrown: + if (stat(opt.log_dir, &statbuf) != 0) + { + if (errno == ENOENT) + { + mkdir(opt.log_dir, S_IRWXU); + INFO("log directory created");
3) Can you include an fflush after the fprintf so that there is no log content lost in case of abrupt failure: + va_start(args, format); + vfprintf(internal_log_file_fp, format, args); + fprintf(internal_log_file_fp, "\n"); + va_end(args);
4) Since you are closing the log file early, the logs after this point like the drop publication/drop replication slot in error flow will be lost. They will neither appear in the console nor in the log file: * Clean up objects created by pg_createsubscriber. @@ -212,6 +315,9 @@ cleanup_objects_atexit(void) if (success) return;
+ if (internal_log_file_fp != NULL) + fclose(internal_log_file_fp); +
5) Since there is only one caller for this function and not needed by anyone else, this code can be moved to the caller: +static void +populate_timestamp(char *timestr, size_t ts_len) +{ + struct timeval tval; + time_t now; + struct tm tmbuf; + + gettimeofday(&tval, NULL); + + /* + * MSVC's implementation of timeval uses a long for tv_sec, however, + * localtime() expects a time_t pointer. Here we'll assign tv_sec to a + * local time_t variable so that we pass localtime() the correct pointer + * type. + */ + now = tval.tv_sec; + strftime(timestr, ts_len, + "%Y-%m-%d-%H-%M-%S", + localtime_r(&now, &tmbuf)); + /* append microseconds */ + snprintf(timestr + strlen(timestr), ts_len - strlen(timestr), + ".%06u", (unsigned int) (tval.tv_usec)); +}
