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
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? |