Re: Log a warning in pg_createsubscriber for max_slot_wal_keep_size

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
Cc: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Log a warning in pg_createsubscriber for max_slot_wal_keep_size
Date: 2025-01-23 06:59:47
Message-ID: CAHut+PtF+S_B4VduXWtU_q9n1s1-5k5W6stAaP9HKLpK+f=sSg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

======
[1] https://github.com/postgres/postgres/commit/ce1b0f9da03e1cebc293f60b378079b22bd7cc0f

Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-01-23 07:05:19 Re: pg_createsubscriber TAP test wrapping makes command options hard to read.
Previous Message Pavel Stehule 2025-01-23 06:50:23 Re: XMLDocument (SQL/XML X030)