Re: speed up a logical replica setup

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Euler Taveira <euler(at)eulerto(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, Andres Freund <andres(at)anarazel(dot)de>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>
Subject: Re: speed up a logical replica setup
Date: 2024-03-18 13:52:28
Message-ID: 7a970912-0b77-4942-84f7-2c9ca0bc05a5@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Attachment Content-Type Size
0001-Various-improvements.patch text/plain 8.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Anthonin Bonnefoy 2024-03-18 14:00:39 Re: POC: Extension for adding distributed tracing - pg_tracing
Previous Message Sean 2024-03-18 13:51:27 Re: Is there still password max length restrictions in PG?