RE: speed up a logical replica setup

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Noah Misch' <noah(at)leadboat(dot)com>, 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>, 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-06-24 06:47:54
Message-ID: OSBPR01MB2552E3CC05BFA971C9289F99F5D42@OSBPR01MB2552.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Noah,

> pg_createsubscriber fails on a dbname containing a space. Use
> appendConnStrVal() here and for other params in get_sub_conninfo(). See the
> CVE-2016-5424 commits for more background. For one way to test this
> scenario,
> see generate_db() in the pg_upgrade test suite.

Thanks for pointing out. I made a fix patch. Test code was also modified accordingly.

> > +static char *
> > +create_logical_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo)
> > +{
> > + PQExpBuffer str = createPQExpBuffer();
> > + PGresult *res = NULL;
> > + const char *slot_name = dbinfo->replslotname;
> > + char *slot_name_esc;
> > + char *lsn = NULL;
> > +
> > + Assert(conn != NULL);
> > +
> > + pg_log_info("creating the replication slot \"%s\" on database \"%s\"",
> > + slot_name, dbinfo->dbname);
> > +
> > + slot_name_esc = PQescapeLiteral(conn, slot_name, strlen(slot_name));
> > +
> > + appendPQExpBuffer(str,
> > + "SELECT lsn FROM
> pg_catalog.pg_create_logical_replication_slot(%s, 'pgoutput', false, false, false)",
>
> This is passing twophase=false, but the patch does not mention prepared
> transactions. Is the intent to not support workloads containing prepared
> transactions? If so, the documentation should say that, and the tool likely
> should warn on startup if max_prepared_transactions != 0.

IIUC, We decided because it is a default behavior of logical replication. See [1].
+1 for improving a documentation, but not sure it is helpful for adding output.
I want to know opinions from others.

> > +static void
> > +create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo)
> > +{
>
> > + appendPQExpBuffer(str, "CREATE PUBLICATION %s FOR ALL TABLES",
> > + ipubname_esc);
>
> This tool's documentation says it "guarantees that no transaction will be
> lost." I tried to determine whether achieving that will require something
> like the fix from
> https://postgr.es/m/flat/de52b282-1166-1180-45a2-8d8917ca74c6(at)enterprise
> db.com.
> (Not exactly the fix from that thread, since that thread has not discussed the
> FOR ALL TABLES version of its race condition.) I don't know. On the one
> hand, pg_createsubscriber benefits from creating a logical slot after creating
> the publication. That snapbuild.c process will wait for running XIDs. On the
> other hand, an INSERT/UPDATE/DELETE acquires its RowExclusiveLock and
> builds
> its relcache entry before assigning an XID, so perhaps the snapbuild.c process
> isn't enough to prevent that thread's race condition. What do you think?

IIUC, documentation just intended to say that a type of replication will be
switched from stream to logical one, at the certain point. Please give sometime
for analyzing.

[1]: https://www.postgresql.org/message-id/270ad9b8-9c46-40c3-b6c5-3d25b91d3a7d%40app.fastmail.com

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/


Attachment Content-Type Size
0001-pg_createsubscriber-Fix-cases-which-connection-param.patch application/octet-stream 10.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2024-06-24 07:06:21 Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
Previous Message Amit Kapila 2024-06-24 06:38:59 Re: speed up a logical replica setup