public inbox for [email protected]  
help / color / mirror / Atom feed
From: vignesh C <[email protected]>
To: Euler Taveira <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: Fabrízio de Royes Mello <[email protected]>
Subject: Re: speed up a logical replica setup
Date: Thu, 7 Mar 2024 15:06:45 +0530
Message-ID: <CALDaNm3PnOcMcE4LahNKSTs=uK7_H2DVsUyAcEQWmKTeCZLexQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAFcNs+qAMdzd_73gwo3UE_25h+E-sHiV2paxfDgz=xw2y7hogw@mail.gmail.com>
	<TY3PR01MB9889D0FF89D502539340EA03F5432@TY3PR01MB9889.jpnprd01.prod.outlook.com>
	<TY3PR01MB9889F3C46DC76CE77B91FB58F5432@TY3PR01MB9889.jpnprd01.prod.outlook.com>
	<TY3PR01MB98894646568B37FE11E9CC0DF5432@TY3PR01MB9889.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TY3PR01MB98896B904BB259B559622773F5422@TY3PR01MB9889.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYCPR01MB120779A0CC2083C1DA8534BA7F5452@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYCPR01MB12077835F614EF829BB035120F5442@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYCPR01MB120777DED2C72F897EF6E57D1F54E2@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<TYCPR01MB12077646F91423005D657FF20F54D2@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYCPR01MB12077756323B79042F29DDAEDF54C2@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<TYCPR01MB12077A8421685E5515DE408EEF5512@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<CALDaNm2ev8_r4N5Sfa2GjzuzZNz17L0nsX_ZqfcarmS5oa5Szw@mail.gmail.com>
	<TYCPR01MB12077CD333376B53F9CAE7AC0F5562@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYCPR01MB12077121263EFF7680486B635F5212@TYCPR01MB12077.jpnprd01.prod.outlook.com>
	<[email protected]>

On Thu, 7 Mar 2024 at 10:05, Euler Taveira <[email protected]> wrote:
>
> On Wed, Mar 6, 2024, at 7:02 AM, Hayato Kuroda (Fujitsu) wrote:
>
> Thanks for updating the patch!
>
>
> Thanks for the feedback. I'm attaching v26 that addresses most of your comments
> and some issues pointed by Vignesh [1].

Few comments:
1) We are disconnecting database again in error case too, it will lead
to a double free in this scenario,
+       PQclear(res);
+
+       disconnect_database(conn, false);
+
+       if (max_repslots < num_dbs)
+       {
+               pg_log_error("subscriber requires %d replication
slots, but only %d remain",
+                                        num_dbs, max_repslots);
+               pg_log_error_hint("Consider increasing
max_replication_slots to at least %d.",
+                                                 num_dbs);
+               disconnect_database(conn, true);
+       }
+
+       if (max_lrworkers < num_dbs)
+       {
+               pg_log_error("subscriber requires %d logical
replication workers, but only %d remain",
+                                        num_dbs, max_lrworkers);
+               pg_log_error_hint("Consider increasing
max_logical_replication_workers to at least %d.",
+                                                 num_dbs);
+               disconnect_database(conn, true);
+       }

pg_createsubscriber: error: subscriber requires 5 logical replication
workers, but only 4 remain
pg_createsubscriber: hint: Consider increasing
max_logical_replication_workers to at least 5.
free(): double free detected in tcache 2
Aborted

2) We can also check that the primary is not using
synchronous_standby_names, else all the transactions in the primary
will wait infinitely once the standby server is stopped, this could be
added in the documentation:
+/*
+ * Is the primary server ready for logical replication?
+ */
+static void
+check_publisher(struct LogicalRepInfo *dbinfo)
+{
+       PGconn     *conn;
+       PGresult   *res;
+
+       char       *wal_level;
+       int                     max_repslots;
+       int                     cur_repslots;
+       int                     max_walsenders;
+       int                     cur_walsenders;
+
+       pg_log_info("checking settings on publisher");
+
+       conn = connect_database(dbinfo[0].pubconninfo, true);
+
+       /*
+        * If the primary server is in recovery (i.e. cascading replication),
+        * objects (publication) cannot be created because it is read only.
+        */
+       if (server_is_in_recovery(conn))
+       {
+               pg_log_error("primary server cannot be in recovery");
+               disconnect_database(conn, true);
+       }

3) This check is present only for publication, we do not have this in
case of creating a subscription. We can keep both of them similar,
i.e. have the check in both or don't have the check for both
publication and subscription:
+       /* Check if the publication already exists */
+       appendPQExpBuffer(str,
+                                         "SELECT 1 FROM
pg_catalog.pg_publication "
+                                         "WHERE pubname = '%s'",
+                                         dbinfo->pubname);
+       res = PQexec(conn, str->data);
+       if (PQresultStatus(res) != PGRES_TUPLES_OK)
+       {
+               pg_log_error("could not obtain publication information: %s",
+                                        PQresultErrorMessage(res));
+               disconnect_database(conn, true);
+       }
+

4) Few options are missing:
+ <refsynopsisdiv>
+  <cmdsynopsis>
+   <command>pg_createsubscriber</command>
+   <arg rep="repeat"><replaceable>option</replaceable></arg>
+   <group choice="plain">
+    <group choice="req">
+     <arg choice="plain"><option>-d</option></arg>
+     <arg choice="plain"><option>--database</option></arg>
+    </group>
+    <replaceable>dbname</replaceable>
+    <group choice="req">
+     <arg choice="plain"><option>-D</option> </arg>
+     <arg choice="plain"><option>--pgdata</option></arg>
+    </group>
+    <replaceable>datadir</replaceable>
+    <group choice="req">
+     <arg choice="plain"><option>-P</option></arg>
+     <arg choice="plain"><option>--publisher-server</option></arg>
+    </group>
+    <replaceable>connstr</replaceable>
+   </group>
+  </cmdsynopsis>
+ </refsynopsisdiv>

4a) -n, --dry-run
4b) -p, --subscriber-port
4c) -r, --retain
4d) -s, --socket-directory
4e) -t, --recovery-timeout
4f) -U, --subscriber-username

5) typo connnections should be connections
+       <para>
+        The port number on which the target server is listening for
connections.
+        Defaults to running the target server on port 50432 to avoid unintended
+        client connnections.

6) repliation should be replication
+ * Create the subscriptions, adjust the initial location for logical
+ * replication and enable the subscriptions. That's the last step for logical
+ * repliation setup.

7) I did not notice these changes in the latest patch:
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d808aad8b0..08de2bf4e6 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -517,6 +517,7 @@ CreateSeqStmt
 CreateStatsStmt
 CreateStmt
 CreateStmtContext
+CreateSubscriberOptions
 CreateSubscriptionStmt
 CreateTableAsStmt
 CreateTableSpaceStmt
@@ -1505,6 +1506,7 @@ LogicalRepBeginData
 LogicalRepCommitData
 LogicalRepCommitPreparedTxnData
 LogicalRepCtxStruct
+LogicalRepInfo
 LogicalRepMsgType
 LogicalRepPartMapEntry
 LogicalRepPreparedTxnData

Regards,
Vignesh






view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: speed up a logical replica setup
  In-Reply-To: <CALDaNm3PnOcMcE4LahNKSTs=uK7_H2DVsUyAcEQWmKTeCZLexQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox