Horton

Re: speed up a logical replica setup

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

View thread
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Euler Taveira <euler@eulerto.com>, pgsql-hackers@lists.postgresql.org
Date: 2022-03-15T13:51:11Z
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

Attachments

On 21.02.22 13:09, Euler Taveira wrote:
> A new tool called pg_subscriber does this conversion and is tightly 
> integrated
> with Postgres.

Are we comfortable with the name pg_subscriber?  It seems too general.
Are we planning other subscriber-related operations in the future?  If
so, we should at least make this one use a --create option or
something like that.

  doc/src/sgml/ref/pg_subscriber.sgml

Attached is a patch that reorganizes the man page a bit.  I moved the
description of the steps to the Notes section and formatted it
differently.  I think the steps are interesting but not essential for
the using of the program, so I wanted to get them out of the main
description.

  src/bin/pg_subscriber/pg_subscriber.c

+   if (made_temp_replslot)
+   {
+       conn = connect_database(dbinfo[0].pubconninfo, true);
+       drop_replication_slot(conn, &dbinfo[0], temp_replslot);
+       disconnect_database(conn);
+   }

Temp slots don't need to be cleaned up.

+/*
+ * Obtain the system identifier from control file. It will be used to 
compare
+ * if a data directory is a clone of another one. This routine is used 
locally
+ * and avoids a replication connection.
+ */
+static char *
+get_control_from_datadir(const char *datadir)

This could return uint64 directly, without string conversion.
get_sysid_from_conn() could then convert to uint64 internally.

+       {"verbose", no_argument, NULL, 'v'},

I'm not sure if the --verbose option is all that useful.

+       {"stop-subscriber", no_argument, NULL, 1},

This option doesn't seem to be otherwise supported or documented.

+   pub_sysid = pg_malloc(32);
+   pub_sysid = get_sysid_from_conn(dbinfo[0].pubconninfo);
+   sub_sysid = pg_malloc(32);
+   sub_sysid = get_control_from_datadir(subscriber_dir);

These mallocs don't appears to be of any use.

+   dbname_conninfo = pg_malloc(NAMEDATALEN);

This seems wrong.

Overall, this code could use a little bit more structuring.  There are
a lot of helper functions that don't seem to do a lot and are mostly
duplicate runs-this-SQL-command calls.  But the main() function is
still huge.  There is room for refinement.

  src/bin/pg_subscriber/t/001_basic.pl

Good start, but obviously, we'll need some real test cases here also.

  src/bin/initdb/initdb.c
  src/bin/pg_ctl/pg_ctl.c
  src/common/file_utils.c
  src/include/common/file_utils.h

I recommend skipping this refactoring.  The readfile() function from
pg_ctl is not general enough to warrant the pride of place of a
globally available function.  Note that it is specifically geared
toward some of pg_ctl's requirements, for example that the underlying
file can change while it is being read.

The requirements of pg_subscriber can be satisfied more easily: Just
call pg_ctl to start the server.  You are already using that in
pg_subscriber.  Is there a reason it can't be used here as well?