Re: speed up a logical replica setup

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Euler Taveira <euler(at)eulerto(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
Subject: Re: speed up a logical replica setup
Date: 2024-02-19 09:47:37
Message-ID: 3ee79f2c-e8b3-4342-857c-a31b87e1afda@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2024-02-19 09:49:24 Re: Fix race condition in InvalidatePossiblyObsoleteSlot()
Previous Message Daniel Gustafsson 2024-02-19 09:27:58 numeric_big in make check?