Re: speed up a logical replica setup

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Euler Taveira <euler(at)eulerto(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, 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>
Subject: Re: speed up a logical replica setup
Date: 2024-03-19 15:24:19
Message-ID: CALDaNm01cKLMeGYWNGO+YVyB246On=m6BW3-TEbcU+otTT8Wzw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 18 Mar 2024 at 16:36, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 18.03.24 08:18, vignesh C wrote:
> > 1) Maximum size of the object name is 64, we can have a check so that
> > we don't specify more than the maximum allowed length:
> > + case 3:
> > + if (!simple_string_list_member(&opt.replslot_names, optarg))
> > + {
> > + simple_string_list_append(&opt.replslot_names, optarg);
> > + num_replslots++;
> > + }
> > + else
> > + {
> > + pg_log_error("duplicate replication slot \"%s\"", optarg);
> > + exit(1);
> > + }
> > + break;
> >
> > If we allow something like this:
> > ./pg_createsubscriber -U postgres -D data_N2/ -P "port=5431
> > user=postgres" -p 5432 -s /home/vignesh/postgres/inst/bin/ -d db1 -d
> > db2 -d db3 --replication-slot="testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes1"
> > --replication-slot="testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes2"
> > --replication-slot="testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes3"
> > In this case creation of replication slot will fail:
> > pg_createsubscriber: error: could not create replication slot
> > "testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes" on
> > database "db2": ERROR: replication slot
> > "testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes"
> > already exists
>
> I think this is fine. The server can check whether the names it is
> given are of the right size. We don't need to check it again in the client.
>
> > 2) Similarly here too:
> > + case 4:
> > + if (!simple_string_list_member(&opt.sub_names, optarg))
> > + {
> > + simple_string_list_append(&opt.sub_names, optarg);
> > + num_subs++;
> > + }
> > + else
> > + {
> > + pg_log_error("duplicate subscription \"%s\"", optarg);
> > + exit(1);
> > + }
> > + break;
> >
> > If we allow something like this:
> > ./pg_createsubscriber -U postgres -D data_N2/ -P "port=5431
> > user=postgres" -p 5432 -s /home/vignesh/postgres/inst/bin/ -d db1 -d
> > db2 -d db3 --subscription=testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes1
> > --subscription=testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes2
> > --subscription=testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes3
> >
> > Subscriptions will be created with the same name and later there will
> > be a problem when setting replication progress as there will be
> > multiple subscriptions with the same name.
>
> Could you clarify this problem?

In this case the subscriptions name specified is more than the allowed
name, the subscription name will be truncated and both the
subscription for db1 and db2 will have same name like below:
db2=# select subname, subdbid from pg_subscription;
subname | subdbid
-----------------------------------------------------------------+---------
testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes | 16384
testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttes | 16385

Now we try to set the replication origin of the subscriptions to a
consistent lsn from the following:
+set_replication_progress(PGconn *conn, struct LogicalRepInfo *dbinfo,
const char *lsn)
+{
+ PQExpBuffer str = createPQExpBuffer();
+ PGresult *res;
+ Oid suboid;
+ char originname[NAMEDATALEN];
+ char lsnstr[17 + 1]; /* MAXPG_LSNLEN = 17 */
+
+ Assert(conn != NULL);
+
+ appendPQExpBuffer(str,
+ "SELECT oid FROM
pg_catalog.pg_subscription "
+ "WHERE subname = '%s'",
+ dbinfo->subname);
+
+ res = PQexec(conn, str->data);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ pg_log_error("could not obtain subscription OID: %s",
+ PQresultErrorMessage(res));
+ disconnect_database(conn, true);
+ }
+
+ if (PQntuples(res) != 1 && !dry_run)
+ {
+ pg_log_error("could not obtain subscription OID: got
%d rows, expected %d rows",
+ PQntuples(res), 1);
+ disconnect_database(conn, true);
+ }

Since the subscription name is truncated, we will have multiple
records returned for the above query which results in failure with:
pg_createsubscriber: error: could not obtain subscription OID: got 2
rows, expected 1 rows
pg_createsubscriber: warning: pg_createsubscriber failed after the end
of recovery
pg_createsubscriber: hint: The target server cannot be used as a
physical replica anymore.
pg_createsubscriber: hint: You must recreate the physical replica
before continuing.

The problem with this failure is that standby has been promoted
already and we will have to re-create the physica replica again.

If you are not planning to have the checks for name length, this could
alternatively be fixed by including database id also while querying
pg_subscription like below in set_replication_progress function:
appendPQExpBuffer(str,
"SELECT oid FROM pg_catalog.pg_subscription \n"
"WHERE subname = '%s' AND subdbid = (SELECT oid FROM
pg_catalog.pg_database WHERE datname = '%s')",
dbinfo->subname,
dbinfo->dbname);

I have verified this fixes the issue.

Regards,
Vignesh

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2024-03-19 15:28:41 Re: Table AM Interface Enhancements
Previous Message Tomas Vondra 2024-03-19 15:22:46 Re: pg_upgrade --copy-file-range