public inbox for [email protected]  
help / color / mirror / Atom feed
From: Noah Misch <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Fabrízio de Royes Mello <[email protected]>
Cc: vignesh C <[email protected]>
Subject: Re: speed up a logical replica setup
Date: Mon, 24 Jun 2024 15:08:45 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA4eK1JGyqFXfvXD+Z81WNz=gOSq1_Ze8MqdLE9PMvTWnVpuZA@mail.gmail.com>
References: <[email protected]>
	<CAA4eK1JtnzqYNQ_S+mj_XsPfgk7szxegKiVQq4-8fY1wUcUnpg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CANhcyEWdTTO71v+GFYUTsOOUvHus1qccg88bRmsU9KykJqjSrQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAA4eK1JGyqFXfvXD+Z81WNz=gOSq1_Ze8MqdLE9PMvTWnVpuZA@mail.gmail.com>

On Mon, Jun 24, 2024 at 05:20:21PM +0530, Amit Kapila wrote:
> On Sun, Jun 23, 2024 at 11:52 AM Noah Misch <[email protected]> wrote:
> >
> > > +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/[email protected].
> > (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

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 you 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 logical
> decoding w.r.t concurrent DDL (Alter Publication ... Add Table). The
> problem happens because we use the old cache state.

Right.  Taking the example from
http://postgr.es/m/[email protected], LSNs
between what that mail calls 4) and 5) are not safely usable as start points.
pg_createsubscriber evades that 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 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 PUBLICATION
committed before pg_create_logical_replication_slot() started.  Second, (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 PUBLICATION
started and managed to assign an XID only after consistent_lsn.  From the
pgoutput perspective, that's indistinguishable from the transaction starting
at its first WAL record, after consistent_lsn.  The linked "long-standing data
loss bug in initial sync of logical replication" thread doesn't have (a),
hence its bug.  How close is that to accurate?

Thanks,
nm






view thread (107+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: speed up a logical replica setup
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox