Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sMGsi-001RRF-M9 for pgsql-hackers@arkaria.postgresql.org; Wed, 26 Jun 2024 00:51:29 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1sMGsg-001GYQ-Ol for pgsql-hackers@arkaria.postgresql.org; Wed, 26 Jun 2024 00:51:27 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sMGsf-001GYH-T5 for pgsql-hackers@lists.postgresql.org; Wed, 26 Jun 2024 00:51:26 +0000 Received: from fhigh5-smtp.messagingengine.com ([103.168.172.156]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sMGsc-0038i3-Gq for pgsql-hackers@lists.postgresql.org; Wed, 26 Jun 2024 00:51:24 +0000 Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 68ED911400CD; Tue, 25 Jun 2024 20:51:21 -0400 (EDT) Received: from imap49 ([10.202.2.99]) by compute2.internal (MEProxy); Tue, 25 Jun 2024 20:51:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eulerto.com; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1719363081; x=1719449481; bh=8ebJ1TLypB kN8sg9B2hQkFHHHnD6GTnVJDeaiWP1TRM=; b=EqJ0aoD9weDnBLAsvyamZdH5Lr wLjApVEaJo2sPmMUi1d1eS1WvNf09/NRPriUPdIGFLLwBMek/Bze3jQTSp3GEDzE c4iyMnhcD63DPVelmBZIiDQlMHwokKIxUyN/aR2tHnhO4bPvGFIuAhOPy32TcNXY kRlQAz3Es3d83qTRRpFkadPM0gnSOqn8ldJd3h+2Ig4ja1yyBl8Yk7m7Cj3kS5jI JHTgqQhXRZb06carHOpOABhtaSbQqNWJeoElwc3ahxphUcGmt33CmCYs6+JUJLCH 9U2JGA/xeysFC4i0eVsxsV3GdyHHzv02uJ7wEFvTyRwPY+BVcTpBMHR11tGQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1719363081; x=1719449481; bh=8ebJ1TLypBkN8sg9B2hQkFHHHnD6 GTnVJDeaiWP1TRM=; b=tadCcU+dlZNTJ/0TQIAzGmIOH0AwB9sDwE779DzxBKk9 2b9DPoH1M2lu/PKh3CZm3qs4JSaMtWwJY7abheIClMM1rMhI2+kLrVoeOJ4cyalc bko+gNyABWHCH9QKv2XrXcIpitf8QdtPM/b0N+aoZ0mDVi9zJQXP1XgiPNasYAv/ WQcDRfgy15YH3QkDfxCCyQM6m1oe2ULSMXiD90EtXw8ysGGFaDgr9XzDHvlXgj/H Sk4p0eljz1UmFI1JuufgpPS6rjte4/nAJKTUlIJuU2qc28aEzluejkf7QNLPYm0z Cd+RUxXTnfWwNIbH+IwAXwPmSvZ4GoKBgBxm2+xMMg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrtddugdeflecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefofgggkfgjfhffhffvvefutgesrgdtreerreerjeenucfhrhhomhepfdfguhhl vghrucfvrghvvghirhgrfdcuoegvuhhlvghrsegvuhhlvghrthhordgtohhmqeenucggtf frrghtthgvrhhnpefhtdelteekffdtveejueevgeeigfdvveeftefhfedvleejueelgfet keevkeduueenucffohhmrghinhepphhoshhtghhrrdgvshdpvghnthgvrhhprhhishgvug gsrdgtohhmnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhho mhepvghulhgvrhesvghulhgvrhhtohdrtghomh X-ME-Proxy: Feedback-ID: i0c21471d:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id A9E2115A0093; Tue, 25 Jun 2024 20:51:20 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.11.0-alpha0-538-g1508afaa2-fm-20240616.001-g1508afaa MIME-Version: 1.0 Message-Id: In-Reply-To: References: <2f0b73f3-e7c0-4e95-9712-1993442a6b64@app.fastmail.com> <34637e7f-0330-420d-8f45-1d022962d2fe@app.fastmail.com> <7a970912-0b77-4942-84f7-2c9ca0bc05a5@eisentraut.org> <44de4e04-db22-40ed-8ba9-46387dc8bf83@app.fastmail.com> <3fa9ef0f-b277-4c13-850a-8ccc04de1406@eisentraut.org> <20240623062157.97.nmisch@google.com> <20240624220845.ba.nmisch@google.com> Date: Tue, 25 Jun 2024 21:50:59 -0300 From: "Euler Taveira" To: "Amit Kapila" , "Noah Misch" Cc: "Peter Eisentraut" , "Shlok Kyal" , "kuroda.hayato@fujitsu.com" , "Tomas Vondra" , "pgsql-hackers@lists.postgresql.org" , "Michael Paquier" , "Andres Freund" , "Ashutosh Bapat" , =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= , "vignesh C" Subject: Re: speed up a logical replica setup Content-Type: multipart/alternative; boundary=f662f171b6f34677858d14d2a43d4566 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --f662f171b6f34677858d14d2a43d4566 Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable On Tue, Jun 25, 2024, at 3:24 AM, Amit Kapila wrote: > On Tue, Jun 25, 2024 at 3:38=E2=80=AFAM Noah Misch = wrote: > > > > On Mon, Jun 24, 2024 at 05:20:21PM +0530, Amit Kapila wrote: > > > On Sun, Jun 23, 2024 at 11:52=E2=80=AFAM Noah Misch wrote: > > > > > > > > > +static void > > > > > +create_publication(PGconn *conn, struct LogicalRepInfo *dbinf= o) > > > > > +{ > > > > > > > > > + appendPQExpBuffer(str, "CREATE PUBLICATION %s FOR ALL TA= BLES", > > > > > + ipubname_esc); > > > > > > > > This tool's documentation says it "guarantees that no transactio= n 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@en= terprisedb.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. O= n 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 RowExclusiveLoc= k and builds > > > > its relcache entry before assigning an XID, so perhaps the snapb= uild.c process > > > > Correction: it doesn't matter how the original INSERT/UPDATE/DELETE = builds its > > relcache entry, just how pgoutput of the change builds the relcache = entry from > > the historic snapshot. > > > > > > isn't enough to prevent that thread's race condition. What do y= ou think? > > > > > > I am not able to imagine how the race condition discussed in the > > > thread you quoted can impact this patch. The problem discussed is > > > mainly the interaction when we are processing the changes in logic= al > > > decoding w.r.t concurrent DDL (Alter Publication ... Add Table). T= he > > > problem happens because we use the old cache state. > > > > Right. Taking the example from > > http://postgr.es/m/20231119021830.d6t6aaxtrkpn743y@awork3.anarazel.d= e, LSNs > > between what that mail calls 4) and 5) are not safely usable as star= t points. > > pg_createsubscriber evades that thread's problem if the consistent_l= sn it > > passes to pg_replication_origin_advance() can't be in a bad-start-po= int LSN > > span. I cautiously bet the snapbuild.c process achieves that: > > > > > I am missing your > > > point about the race condition mentioned in the thread you quoted = with > > > snapbuild.c. Can you please elaborate a bit more? > > > > When pg_createsubscriber calls pg_create_logical_replication_slot(),= the key > > part starts at: > > > > /* > > * If caller needs us to determine the decoding start point,= do so now. > > * This might take a while. > > */ > > if (find_startpoint) > > DecodingContextFindStartpoint(ctx); > > > > Two factors protect pg_createsubscriber. First, (a) CREATE PUBLICAT= ION > > committed before pg_create_logical_replication_slot() started. Seco= nd, (b) > > DecodingContextFindStartpoint() waits for running XIDs to complete, = via the > > process described at the snapbuild.c "starting up in several stages"= diagram. > > Hence, the consistent_lsn is not in a bad-start-point LSN span. It'= s fine > > even if the original INSERT populated all caches before CREATE PUBLI= CATION > > started and managed to assign an XID only after consistent_lsn. Fro= m the > > pgoutput perspective, that's indistinguishable from the transaction = starting > > at its first WAL record, after consistent_lsn. The linked "long-sta= nding data > > loss bug in initial sync of logical replication" thread doesn't have= (a), > > hence its bug. How close is that to accurate? > > >=20 > Yeah, this theory sounds right to me. The key point is that no DML > (processing of WAL corresponding to DML) before CREATE PUBLICATION ... > command would have reached pgoutput level because we would have waited > for it during snapbuild.c. Can we conclude that the race condition > discussed in the other thread won't impact this patch? As Noah said the key point is the CREATE PUBLICATION *before* creating t= he replication slots -- that wait transactions to complete. -- Euler Taveira EDB https://www.enterprisedb.com/ --f662f171b6f34677858d14d2a43d4566 Content-Type: text/html;charset=utf-8 Content-Transfer-Encoding: quoted-printable
On Tue, Jun 25,= 2024, at 3:24 AM, Amit Kapila wrote:
On Tue, Jun 25, 2024 at 3:38=E2=80=AFAM Noah = Misch <noah@leadboat.com>= wrote:
>
> On Mon, Jun 24, 2024 at 05= :20:21PM +0530, Amit Kapila wrote:
> > On Sun, Jun 2= 3, 2024 at 11:52=E2=80=AFAM Noah Misch <noah@leadboat.com> wrote:
> > >
> > > > +static void
> > >= ; > +create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo)<= br>
> > > > +{
> > >
> > > > +     appendPQExpBuffer(= str, "CREATE PUBLICATION %s FOR ALL TABLES",
> > >= ; > +          &nbs= p;           &nbs= p;           &nbs= p;    ipubname_esc);
> > >
> > > This tool's documentation says it "guarantees that= no transaction will be
> > > lost."  I trie= d to determine whether achieving that will require something
> > > like the fix from
> > > https://postgr.es/m/flat/de52b282-1166-1180-45a2-8d8917= ca74c6@enterprisedb.com.
> > > (Not exactly t= he fix from that thread, since that thread has not discussed the
> > > FOR ALL TABLES version of its race condition.)&nbs= p; I don't know.  On the one
> > > hand, pg_= createsubscriber benefits from creating a logical slot after creating
> > > the publication.  That snapbuild.c proce= ss will wait for running XIDs.  On the
> > >= other hand, an INSERT/UPDATE/DELETE acquires its RowExclusiveLock and b= uilds
> > > its relcache entry before assigning a= n XID, so perhaps the snapbuild.c process
>
> Correction: it doesn't matter how the original INSERT/UPDATE/DEL= ETE builds its
> relcache entry, just how pgoutput of t= he change builds the relcache entry from
> the historic= snapshot.
>
> > > isn't enough = to prevent that thread's race condition.  What do you think?
> >
> > I am not able to imagine how t= he race condition discussed in the
> > thread you qu= oted can impact this patch. The problem discussed is
> = > mainly the interaction when we are processing the changes in logica= l
> > decoding w.r.t concurrent DDL (Alter Publicati= on ... Add Table). The
> > problem happens because w= e use the old cache state.
>
> Right.&= nbsp; Taking the example from
> between what that mail calls 4) and 5) are not safely= usable as start points.
> pg_createsubscriber evades t= hat thread's problem if the consistent_lsn it
> passes = to pg_replication_origin_advance() can't be in a bad-start-point LSN
=
> span.  I cautiously bet the snapbuild.c process ach= ieves that:
>
> > I am missing your=
> > point about the race condition mentioned in the= thread you quoted with
> > snapbuild.c. Can you ple= ase elaborate a bit more?
>
> When pg_= createsubscriber calls pg_create_logical_replication_slot(), the key
=
> part starts at:
>
>&nb= sp;        /*
> = ;         * If caller needs us t= o determine the decoding start point, do so now.
> = ;         * This might take a wh= ile.
>        &= nbsp; */
>       &nb= sp; if (find_startpoint)
>     = ;            Deco= dingContextFindStartpoint(ctx);
>
> Tw= o factors protect pg_createsubscriber.  First, (a) CREATE PUBLICATI= ON
> committed before pg_create_logical_replication_slo= t() started.  Second, (b)
> DecodingContextFindSta= rtpoint() waits for running XIDs to complete, via the
>= process described at the snapbuild.c "starting up in several stages" di= agram.
> Hence, the consistent_lsn is not in a bad-star= t-point LSN span.  It's fine
> even if the origina= l INSERT populated all caches before CREATE PUBLICATION
&g= t; started and managed to assign an XID only after consistent_lsn. = From the
> pgoutput perspective, that's indistinguisha= ble from the transaction starting
> at its first WAL re= cord, after consistent_lsn.  The linked "long-standing data
> loss bug in initial sync of logical replication" thread does= n't have (a),
> hence its bug.  How close is that = to accurate?
>

Yeah, this = theory sounds right to me. The key point is that no DML
(p= rocessing of WAL corresponding to DML) before CREATE PUBLICATION ...
=
command would have reached pgoutput level because we would ha= ve waited
for it during snapbuild.c. Can we conclude that = the race condition
discussed in the other thread won't imp= act this patch?

As Noah said t= he key point is the CREATE PUBLICATION *before* creating the
replication slots -- that wait transactions to complete.


--
Euler Taveira

--f662f171b6f34677858d14d2a43d4566--