Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.
Date: 2025-03-19 08:58:58
Message-ID: CALDaNm3GhvZ-8o+7QE6jMLocTW7+kMbJAvBPW-idYR4tGEDJCw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 19 Mar 2025 at 11:44, Shubham Khanna
<khannashubham1197(at)gmail(dot)com> wrote:
>
>
> I agree with you on this; switching to a single query with safe_psql()
> will indeed simplify the test and make the verification logic cleaner.
>
> The attached patch contains the suggested change.

Few comments:
1) I felt we are not doing anything specific to dry-run, so no need to
have this test case:
+# run pg_createsubscriber with '--all' and '--database' and verify the
+# failure
+command_fails_like(
+ [
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--all',
+ '--database' => $db1,
+ ],
+ qr/--database cannot be used with --all/,
+ 'fail if --database is used with --all');
+

2) Similarly this too:
+# run pg_createsubscriber with '--all' option
+command_ok(
+ [
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--all',
+ ],
+ 'run pg_createsubscriber with --all');
+

3) We have run other pg_createsubscriber success tests with
recovery-timeout specified, so I suggest modifying this too similarly:
+# run pg_createsubscriber with '--all' option without '--dry-run'
+command_ok(
+ [
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $node_u->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_u->host,
+ '--subscriber-port' => $node_u->port,
+ '--all',
+ ],
+ 'run pg_createsubscriber with --all');
+
+$node_u->start;

4) You can try to see how much extra time it takes to run the tests,
if takes more time, then you can think of the following changes a) few
of the option validation test cases can be removed, we can have just
one combination of all with either of
publication/subscription/replication slot b) for the last test added,
you can drop one of the dbs and verify the subscriptions are created
in 2 dbs as the code flow is the same for all databases.

Regards,
Vignesh

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Shubham Khanna 2025-03-19 09:01:41 Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.
Previous Message Christophe Pettus 2025-03-19 08:53:37 Vacuuming the free space map considered harmful?