Horton

Re: speed up a logical replica setup

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Euler Taveira <euler@eulerto.com>, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, vignesh C <vignesh21@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Fabrízio de Royes Mello <fabriziomello@gmail.com>
Date: 2024-02-19T09:47:37Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. pg_createsubscriber: Remove obsolete comment

  2. pg_createsubscriber: Fix an unpredictable recovery wait time.

  3. Fix unstable test in 040_pg_createsubscriber.

  4. Fix the testcase introduced in commit 81d20fbf7a.

  5. Further weaken new pg_createsubscriber test on Windows.

  6. Temporarily(?) weaken new pg_createsubscriber test on Windows.

  7. Make pg_createsubscriber warn if publisher has two-phase commit enabled.

  8. Make pg_createsubscriber more wary about quoting connection parameters.

  9. pg_createsubscriber: Remove failover replication slots on subscriber

  10. pg_createsubscriber: Remove replication slot check on primary

  11. pg_createsubscriber: Only --recovery-timeout controls the end of recovery process

  12. pg_createsubscriber: creates a new logical replica from a standby server

  13. Add some const decorations

  14. Add option force_initdb to PostgreSQL::Test::Cluster:init()

  15. Remove MSVC scripts

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?