Re: [Proposal] Adding Log File Capability to pg_createsubscriber

On Mon, 2 Feb 2026 at 04:35, Gyan Sreejith <gyan(dot)sreejith(at)gmail(dot)com> wrote:
>
> Thank you!
>
> I have made the suggested changes. In addition, I added a wrapper for pg_fatal to write to the file and then do everything that pg_fatal would do.
> I have attached the patch.

Few comments:
1) Can we change the macro names:
INFO -> pg_log_info
INFO_HINT -> pg_log_info_hint
DEBUG -> pg_log_debug
FATAL -> pg_fatal

if required do an #undef and call pg_log_generic in the else similar
to how it is done in pg_backup_utils.h

+#define INFO(...) do{\
+ if (internal_log_file_fp != NULL) \
+ internal_log_file_write(__VA_ARGS__); \
+ else \
+ pg_log_info(__VA_ARGS__);\
+} while(0)
+
+#define INFO_HINT(...) do{\
+ if (internal_log_file_fp != NULL) \
+ internal_log_file_write(__VA_ARGS__); \
+ else \
+ pg_log_info_hint(__VA_ARGS__);\
+} while(0)
+
+#define DEBUG(...) do{\
+ if (internal_log_file_fp != NULL) \
+ internal_log_file_write(__VA_ARGS__); \
+ else \
+ pg_log_debug(__VA_ARGS__);\
+} while(0)
+
+#define FATAL(...) do{\
+ if (internal_log_file_fp != NULL) \
+ internal_log_file_write(__VA_ARGS__); \
+ pg_fatal(__VA_ARGS__); /* call pg_fatal after writing to logs */ \
+} while(0)

2) You can keep the variabled in case 'l' handling to keep the scope
accordingly:
+ char timestamp[128];
+ struct timeval tval;
+ time_t now;

3) Instead of creating a new standby and running with -l option, can
you run it with -l for one of the existing tests:
+$node_p->backup('backup_3');
+
+# Set up node R as a logical replica node
+my $node_r = PostgreSQL::Test::Cluster->new('node_r');
+$node_r->init_from_backup($node_p, 'backup_3', has_streaming => 1);
+$node_r->append_conf(
+ 'postgresql.conf', qq[
+primary_conninfo = '$pconnstr dbname=postgres'
+hot_standby_feedback = on
+]);
+$node_r->set_standby_mode();
+
+# Test that --logdir works for pg_createsubscriber
+command_ok(
+ [
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $node_r->data_dir,
+ '--publisher-server' => $pconnstr,
+ '--database' => 'postgres',
+ '--logdir' => $logdir,
+ ],
+ 'check for log file creation for pg_createSubscriber');
+
+# Check that the log files were created
+my @server_log_files = glob "$logdir/pg_createsubscriber_server_*.log";
+is( scalar(@server_log_files), 1, "
+ pg_createsubscriber_server.log file was created");
+my @internal_log_files = glob "$logdir/pg_createsubscriber_internal_*.log";
+is( scalar(@internal_log_files), 1, "
+ pg_createsubscriber_internal.log file was created");

Regards,
Vignesh