Re: Log a warning in pg_createsubscriber for max_slot_wal_keep_size

On Thu, Jan 23, 2025 at 12:30 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Shubham.
>
> The v12-0001 works OK, but your added TAP tests of this patch do not
> conform to the new style of the fat-comma parameter passing since the
> recent commit [1], so you'll need to fix them so they do...
>
> ======
> src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
>
> 1.
> +command_checks_all(
> + [
> + 'pg_createsubscriber', '--verbose',
> + '--verbose', '--recovery-timeout',
> + "$PostgreSQL::Test::Utils::timeout_default", '--dry-run',
> + '--pgdata', $node_s->data_dir,
> + '--publisher-server', $node_p->connstr($db1),
> + '--socketdir', $node_s->host,
> + '--subscriber-port', $node_s->port,
> + '--publication', 'pub1',
> + '--publication', 'pub2',
> + '--subscription', 'sub1',
> + '--subscription', 'sub2',
> + '--database', $db1,
> + '--database', $db2
> + ],
>
>
> 1a.
> Fix the formatting. e.g. Look at master for example what new style
> parameters look like and make yours look the same.
>
> ~
>
> 1b.
> Here I think we don't really any --verbose at all because AFAIK
> command_checks_all is not producing any output we can look at anyhow.
>
> ~~~
>
> 2.
> +# And, same again to see the output...
> command_ok(
> [
> - 'pg_createsubscriber',
> - '--verbose',
> - '--dry-run',
> - '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default,
> - '--pgdata' => $node_s->data_dir,
> - '--publisher-server' => $node_p->connstr($db1),
> - '--socketdir' => $node_s->host,
> - '--subscriber-port' => $node_s->port,
> - '--publication' => 'pub1',
> - '--publication' => 'pub2',
> - '--subscription' => 'sub1',
> - '--subscription' => 'sub2',
> - '--database' => $db1,
> - '--database' => $db2,
> + 'pg_createsubscriber', '--verbose',
> + '--verbose', '--recovery-timeout',
> + "$PostgreSQL::Test::Utils::timeout_default", '--dry-run',
> + '--pgdata', $node_s->data_dir,
> + '--publisher-server', $node_p->connstr($db1),
> + '--socketdir', $node_s->host,
> + '--subscriber-port', $node_s->port,
> + '--publication', 'pub1',
> + '--publication', 'pub2',
> + '--subscription', 'sub1',
> + '--subscription', 'sub2',
> + '--database', $db1,
> + '--database', $db2
> ],
>
> 2a.
> +# And, same again to see the output...
>
> Maybe a better comment is more like
>
> # Execute the same command again but this time use 'command_ok' so
> that the log files can be inspected.
>
> ~
>
> 2b.
> Fix the formatting. Look at master for example what new style
> parameters look like and make yours look the same.
>
> ~
>
> 2c.
> Include some comment (like exists elsewhere in this file) to say
> "--verbose is used twice to show more information.", otherwise readers
> could be tempted to think the double --verbose is a mistake.
>
> ~~~
>
> 3.
> Rerun perltidy (the correct version) to ensure the formatting is
> stable and good.
>
> ======

Fixed the given comments. The attached patch contains the suggested changes.

Thanks and regards,
Shubham Khanna.