From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Shubham Khanna' <khannashubham1197(at)gmail(dot)com> |
Cc: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Peter Smith <smithpb2250(at)gmail(dot)com> |
Subject: | RE: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility. |
Date: | 2025-02-25 11:20:28 |
Message-ID: | OSCPR01MB14966189AEFD119DD04FE6B42F5C32@OSCPR01MB14966.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Shubham,
Thanks for updating the patch. Few comments.
01. CreateSubscriberOptions
```
+ bool cleanup_existing_publications; /* drop all publications */
```
My previous comment [1] #1 did not intend to update attributes. My point was
only for the third argument of setup_subscriber().
02. setup_subscriber()
```
+ pg_log_info("cleaning up existing publications on the subscriber");
```
I don't think this output is needed. check_and_drop_existing_publications() has the similar output.
03. drop_publication_by_name()
```
+
+ appendPQExpBuffer(query, "DROP PUBLICATION %s", pubname_esc);
+ pg_log_info("dropping publication %s in database \"%s\"", pubname, dbname);
+ pg_log_debug("command is: %s", query->data);
```
Currently, appendPQExpBuffer() is done after the pg_log_info(). Please reserve current ordering if
there are no reasons. Also, variable "str" is currently used instead of query, please follow.
04. drop_publication_by_name()
```
if (!dry_run)
{
- res = PQexec(conn, str->data);
+ res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ dbinfo->made_publication = false;
+ else
{
- pg_log_error("could not drop publication \"%s\" in database \"%s\": %s",
- dbinfo->pubname, dbinfo->dbname, PQresultErrorMessage(res));
- dbinfo->made_publication = false; /* don't try again. */
-
- /*
- * Don't disconnect and exit here. This routine is used by primary
- * (cleanup publication / replication slot due to an error) and
- * subscriber (remove the replicated publications). In both cases,
- * it can continue and provide instructions for the user to remove
- * it later if cleanup fails.
- */
+ pg_log_error("could not drop publication %s in database \"%s\": %s",
+ pubname, dbname, PQresultErrorMessage(res));
}
```
pg_log_error() is exected when the command succeed: This is not correct.
05. 040_pg_createsubscriber.pl
```
+# Set up node A as primary
+my $node_a = PostgreSQL::Test::Cluster->new('node_a');
+my $aconnstr = $node_a->connstr;
+$node_a->init(allows_streaming => 'logical');
+$node_a->append_conf('postgresql.conf', 'autovacuum = off');
+$node_a->start;
```
I don't think new primary is not needed. Can't you reuse node_p?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
From | Date | Subject | |
---|---|---|---|
Next Message | Vladlen Popolitov | 2025-02-25 11:21:12 | Re: SQL Property Graph Queries (SQL/PGQ) |
Previous Message | John Naylor | 2025-02-25 11:05:17 | Re: Improve CRC32C performance on SSE4.2 |