MIME-Version: 1.0
References: <CAJpy0uDhR=m2Pr76aJZnvtKZpCEiLbMbjbPReCv5fS0MA3ch2w@mail.gmail.com>
 <CAJpy0uDWgSBUsTav1Eeo7E7i38GjXr_o0WdWamm3jGdctcoC4A@mail.gmail.com>
 <ZZa4pLFCe2mAks1m@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAJpy0uBGSjwhZTBo6MxT6TkwO2AX5fa1kw7UC3RQLiapUJReuw@mail.gmail.com>
 <CAFiTN-vTaWUh0Eq8UpETMk=ZGFEMkD-1gXtosODZeCiCfi92kQ@mail.gmail.com>
 <CAA4eK1KLR2BP1GULkA9FMj3aKFbOcQLvu6b5fAht+YjCVRkc1w@mail.gmail.com>
 <CAFiTN-skn=ffyC8PMzg7gfpyNLfx7v9roae-+W8HYbpzoO0mkg@mail.gmail.com>
 <OS0PR01MB57169DD55EC8D9D1EDB7A0C2946A2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAA4eK1L74EUU7+Dr11joY+Xact_MsjZu9G12sX+vRUWJsptP8Q@mail.gmail.com>
 <CAA4eK1+ORhnb2zUUpO_FcgWUUbwvQX5+=tdCehx7x2nB2x8VMg@mail.gmail.com> <ZaAMOwuYPc+Aaf/I@ip-10-97-1-34.eu-west-3.compute.internal>
In-Reply-To: <ZaAMOwuYPc+Aaf/I@ip-10-97-1-34.eu-west-3.compute.internal>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Fri, 12 Jan 2024 08:42:39 +0530
Message-ID: <CAA4eK1LDzUJDt3eeuGLh78fEvN8aJ_c86KW8c+hncPdt9_7xxw@mail.gmail.com>
Subject: Re: Synchronizing slots from primary to standby
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, 
	Dilip Kumar <dilipbalaut@gmail.com>, shveta malik <shveta.malik@gmail.com>, 
	Peter Smith <smithpb2250@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, 
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, 
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Bruce Momjian <bruce@momjian.us>, 
	Ashutosh Sharma <ashu.coek88@gmail.com>, Andres Freund <andres@anarazel.de>, 
	pgsql-hackers <pgsql-hackers@postgresql.org>, Ajin Cherian <itsajin@gmail.com>, 
	Alvaro Herrera <alvherre@alvh.no-ip.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAA4eK1LDzUJDt3eeuGLh78fEvN8aJ_c86KW8c%2BhncPdt9_7xxw%40mail.gmail.com>
Precedence: bulk

On Thu, Jan 11, 2024 at 9:11=E2=80=AFPM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> > On Tue, Jan 9, 2024 at 6:39=E2=80=AFPM Amit Kapila <amit.kapila16@gmail=
.com> wrote:
> > >
> > > +static bool
> > > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slo=
t)
> > > {
> > > ...
> > > + /* Slot ready for sync, so sync it. */
> > > + else
> > > + {
> > > + /*
> > > + * Sanity check: With hot_standby_feedback enabled and
> > > + * invalidations handled appropriately as above, this should never
> > > + * happen.
> > > + */
> > > + if (remote_slot->restart_lsn < slot->data.restart_lsn)
> > > + elog(ERROR,
> > > + "cannot synchronize local slot \"%s\" LSN(%X/%X)"
> > > + " to remote slot's LSN(%X/%X) as synchronization"
> > > + " would move it backwards", remote_slot->name,
> > > + LSN_FORMAT_ARGS(slot->data.restart_lsn),
> > > + LSN_FORMAT_ARGS(remote_slot->restart_lsn));
> > > ...
> > > }
> > >
> > > I was thinking about the above code in the patch and as far as I can
> > > think this can only occur if the same name slot is re-created with
> > > prior restart_lsn after the existing slot is dropped. Normally, the
> > > newly created slot (with the same name) will have higher restart_lsn
> > > but one can mimic it by copying some older slot by using
> > > pg_copy_logical_replication_slot().
> > >
> > > I don't think as mentioned in comments even if hot_standby_feedback i=
s
> > > temporarily set to off, the above shouldn't happen. It can only lead
> > > to invalidated slots on standby.
>
> I also think so.
>
> > >
> > > To close the above race, I could think of the following ways:
> > > 1. Drop and re-create the slot.
> > > 2. Emit LOG/WARNING in this case and once remote_slot's LSN moves
> > > ahead of local_slot's LSN then we can update it; but as mentioned in
> > > your previous comment, we need to update all other fields as well. If
> > > we follow this then we probably need to have a check for catalog_xmin
> > > as well.
>
> IIUC, this would be a sync slot (so not usable until promotion) that coul=
d
> not be used anyway (invalidated), so I'll vote for drop / re-create then.
>

No, it can happen for non-sync slots as well.

> > > Now, related to this the other case which needs some handling is what
> > > if the remote_slot's restart_lsn is greater than local_slot's
> > > restart_lsn but it is a re-created slot with the same name. In that
> > > case, I think the other properties like 'two_phase', 'plugin' could b=
e
> > > different. So, is simply copying those sufficient or do we need to do
> > > something else as well?
> > >
> >
>
> I'm not sure to follow here. If the remote slot is re-created then it wou=
ld
> be also dropped / re-created locally, or am I missing something?
>

As our slot-syncing mechanism is asynchronous (from time to time we
check the slot information on primary), isn't it possible that the
same name slot is dropped and recreated between slot-sync worker's
checks?

--=20
With Regards,
Amit Kapila.