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>, Amit Kapila <amit.kapila16@gmail.com>
Cc: Shlok Kyal <shlok.kyal.oss@gmail.com>, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, Tomas Vondra <tomas.vondra@enterprisedb.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Fabrízio de Royes Mello <fabriziomello@gmail.com>, vignesh C <vignesh21@gmail.com>
Date: 2024-03-18T13:52:28Z
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 16.03.24 16:42, Euler Taveira wrote:
> I'm attaching a new version (v30) that adds:

I have some review comments and attached a patch with some smaller 
fixups (mainly message wording and avoid fixed-size string buffers).

* doc/src/sgml/ref/pg_createsubscriber.sgml

I would remove the "How It Works" section.  This is not relevant to
users, and it is very detailed and will require updating whenever the
implementation changes.  It could be a source code comment instead.

* src/bin/pg_basebackup/pg_createsubscriber.c

I think the connection string handling is not robust against funny
characters, like spaces, in database names etc.

Most SQL commands need to be amended for proper identifier or string
literal quoting and/or escaping.

In check_subscriber(): All these permissions checks seem problematic
to me.  We shouldn't reimplement our own copy of the server's
permission checks.  The server can check the permissions.  And if the
permission checking in the server ever changes, then we have
inconsistencies to take care of.  Also, the error messages "permission
denied" are inappropriate, because we are not doing the actual thing.
Maybe we want to do a dry-run for the benefit of the user, but then we
should do the actual thing, like try to create a replication slot, or
whatever.  But I would rather just remove all this, it seems too
problematic.

In main(): The first check if the standby is running is problematic.
I think it would be better to require that the standby is initially
shut down.  Consider, the standby might be running under systemd.
This tool will try to stop it, systemd will try to restart it.  Let's
avoid these kinds of battles.  It's also safer if we don't try to
touch running servers.

The -p option (--subscriber-port) doesn't seem to do anything.  In my
testing, it always uses the compiled-in default port.

Printing all the server log lines to the terminal doesn't seem very
user-friendly.  Not sure what to do about that, short of keeping a 
pg_upgrade-style directory of log files.  But it's ugly.