Re: speed up a logical replica setup - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: speed up a logical replica setup
Date
Msg-id 3ee79f2c-e8b3-4342-857c-a31b87e1afda@eisentraut.org
Whole thread Raw
In response to Re: speed up a logical replica setup  ("Euler Taveira" <euler@eulerto.com>)
Responses Re: speed up a logical replica setup
List pgsql-hackers
Some review of the v21 patch:

- commit message

Mention pg_createsubscriber in the commit message title.  That's the 
most important thing that someone doing git log searches in the future 
will be looking for.


- doc/src/sgml/ref/allfiles.sgml

Move the new entry to alphabetical order.


- doc/src/sgml/ref/pg_createsubscriber.sgml

+  <para>
+   The <application>pg_createsubscriber</application> should be run at 
the target
+   server. The source server (known as publisher server) should accept 
logical
+   replication connections from the target server (known as subscriber 
server).
+   The target server should accept local logical replication connection.
+  </para>

"should" -> "must" ?

+ <refsect1>
+  <title>Options</title>

Sort options alphabetically.

It would be good to indicate somewhere which options are mandatory.

+ <refsect1>
+  <title>Examples</title>

I suggest including a pg_basebackup call into this example, so it's
easier for readers to get the context of how this is supposed to be
used.  You can add that pg_basebackup in this example is just an example 
and that other base backups can also be used.


- doc/src/sgml/reference.sgml

Move the new entry to alphabetical order.


- src/bin/pg_basebackup/Makefile

Move the new sections to alphabetical order.


- src/bin/pg_basebackup/meson.build

Move the new sections to alphabetical order.


- src/bin/pg_basebackup/pg_createsubscriber.c

+typedef struct CreateSubscriberOptions
+typedef struct LogicalRepInfo

I think these kinds of local-use struct don't need to be typedef'ed.
(Then you also don't need to update typdefs.list.)

+static void
+usage(void)

Sort the options alphabetically.

+static char *
+get_exec_path(const char *argv0, const char *progname)

Can this not use find_my_exec() and find_other_exec()?

+int
+main(int argc, char **argv)

Sort the options alphabetically (long_options struct, getopt_long()
argument, switch cases).


- .../t/040_pg_createsubscriber.pl
- .../t/041_pg_createsubscriber_standby.pl

These two files could be combined into one.

+# Force it to initialize a new cluster instead of copying a
+# previously initdb'd cluster.

Explain why?

+$node_s->append_conf(
+    'postgresql.conf', qq[
+log_min_messages = debug2

Is this setting necessary for the test?




pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: numeric_big in make check?
Next
From: Bertrand Drouvot
Date:
Subject: Re: Fix race condition in InvalidatePossiblyObsoleteSlot()