Re: speed up a logical replica setup

On Mon, 18 Mar 2024 at 16:36, Peter Eisentraut <peter@eisentraut.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