Re: [Proposal] Adding Log File Capability to pg_createsubscriber

I was able to make the first two changes that you suggested. However, I was
not able to get the test to run with one of the existing replica servers. I
have attached the patch for the first two changes.
Thank you!

Regards,
Gyan

On Wed, Feb 4, 2026 at 4:44 AM vignesh C <vignesh21@gmail.com> wrote:

> On Mon, 2 Feb 2026 at 04:35, Gyan Sreejith <gyan.sreejith@gmail.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
>