Обсуждение: pgsql: pg_createsubscriber: creates a new logical replica from a standb

pg_createsubscriber: creates a new logical replica from a standby server

It must be run on the target server and should be able to connect to
the source server (publisher) and the target server (subscriber).  All
tables in the specified database(s) are included in the logical
replication setup.  A pair of publication and subscription objects are
created for each database.

The main advantage of pg_createsubscriber over the common logical
replication setup is the initial data copy.  It also reduces the
catchup phase.

Some prerequisites must be met to successfully run it.  It is
basically the logical replication requirements.  It starts creating a
publication using FOR ALL TABLES and a replication slot for each
specified database.  Write recovery parameters into the target data
directory and start the target server.  It specifies the LSN of the
last replication slot (replication start point) up to which the
recovery will proceed.  Wait until the target server is promoted.
Create one subscription per specified database (using publication and
replication slot created in a previous step) on the target server.
Set the replication progress to the replication start point for each
subscription.  Enable the subscription for each specified database on
the target server.  And finally, change the system identifier on the
target server.

Author: Euler Taveira <euler.taveira@enterprisedb.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Shubham Khanna <khannashubham1197@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/5ac50071-f2ed-4ace-a8fd-b892cffd33eb@www.fastmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d44032d0146306971cd5ccf232fe37269717d6f2

Modified Files
--------------
doc/src/sgml/ref/allfiles.sgml                     |    1 +
doc/src/sgml/ref/pg_createsubscriber.sgml          |  523 +++++
doc/src/sgml/reference.sgml                        |    1 +
src/bin/pg_basebackup/.gitignore                   |    1 +
src/bin/pg_basebackup/Makefile                     |   11 +-
src/bin/pg_basebackup/meson.build                  |   19 +
src/bin/pg_basebackup/nls.mk                       |    1 +
src/bin/pg_basebackup/pg_createsubscriber.c        | 2141 ++++++++++++++++++++
src/bin/pg_basebackup/t/040_pg_createsubscriber.pl |  364 ++++
9 files changed, 3059 insertions(+), 3 deletions(-)

Hello.

This commit added the following error message:

pg_createsubscriber.c: 375
>            pg_fatal("could not access directory \"%s\": %s", datadir,
>                     strerror(errno));

Although other messages use %s with PQresultErrorMessage(), regarding
this specific message, shouldn't we use %m instead of %s + strerror()?
I'm not sure if that would be better.


pg_createsubscriber.c: 687
>        pg_log_error("could not obtain database OID: got %d rows, expected %d rows",
>                     PQntuples(res), 1);
pg_createsubscriber.c: 1652
>        pg_log_error("could not obtain subscription OID: got %d rows, expected %d rows",

In these messages, the second %d is always written as "1 rows",
whereas a similar message correctly uses "1 row".

pg_createsubscriber.c: 561
>        pg_log_error("could not get system identifier: got %d rows, expected %d row",
>                     PQntuples(res), 1);

I think it would be better to change the former instances to "%d row",
or to change both to "1 row". I'd like to go the second way but maybe
we should take the first way following our convention.


pg_createsubscriber.c: 923
>        pg_log_error("publisher requires wal_level >= logical");

We discussed this message in relation to commit 801792e528, and
decided to quote "logical" to clarify that it is a string literal. I'd
like to follow the conclusion here, too.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

I have committed your patch to tidy this up.  Thanks.

On 26.03.24 06:01, Kyotaro Horiguchi wrote:
> Hello.
> 
> This commit added the following error message:
> 
> pg_createsubscriber.c: 375
>>             pg_fatal("could not access directory \"%s\": %s", datadir,
>>                      strerror(errno));
> 
> Although other messages use %s with PQresultErrorMessage(), regarding
> this specific message, shouldn't we use %m instead of %s + strerror()?
> I'm not sure if that would be better.
> 
> 
> pg_createsubscriber.c: 687
>>         pg_log_error("could not obtain database OID: got %d rows, expected %d rows",
>>                      PQntuples(res), 1);
> pg_createsubscriber.c: 1652
>>         pg_log_error("could not obtain subscription OID: got %d rows, expected %d rows",
> 
> In these messages, the second %d is always written as "1 rows",
> whereas a similar message correctly uses "1 row".
> 
> pg_createsubscriber.c: 561
>>         pg_log_error("could not get system identifier: got %d rows, expected %d row",
>>                      PQntuples(res), 1);
> 
> I think it would be better to change the former instances to "%d row",
> or to change both to "1 row". I'd like to go the second way but maybe
> we should take the first way following our convention.
> 
> 
> pg_createsubscriber.c: 923
>>         pg_log_error("publisher requires wal_level >= logical");
> 
> We discussed this message in relation to commit 801792e528, and
> decided to quote "logical" to clarify that it is a string literal. I'd
> like to follow the conclusion here, too.
> 
> regards.
>