MIME-Version: 1.0
References: <7daef094-abf3-4672-bc23-3df4763b16a3@gmail.com>
 <aiP/X1FThlZRCFiN@bdtpg>
 <OS9PR01MB1214908BA67A7811BD6281208F51C2@OS9PR01MB12149.jpnprd01.prod.outlook.com>
 <CABPTF7WmLKOJcSPod7zT2hynUFQcUs0VoQyR-p=XFsafvmGm7g@mail.gmail.com>
 <CABPTF7U_B9pdC563fFONQLX_FXCZtxZgM+jN9snzMVg9b9MRfg@mail.gmail.com>
 <aibQB2f9h3nYeg/2@bdtpg>
 <CABPTF7XGDgnB97fBUQb+Tm=5NnAzS1iKTWcJTePpGinL1ZAxWQ@mail.gmail.com>
 <CABPTF7VovNWnH=dG_A=e6xkMYt11ubbj935Ymfky55cXcWcrrA@mail.gmail.com>
 <aifyiJG3uiJzCYTB@bdtpg>
In-Reply-To: <aifyiJG3uiJzCYTB@bdtpg>
From: Xuneng Zhou <xunengzhou@gmail.com>
Date: Wed, 10 Jun 2026 16:36:14 +0800
Message-ID: 
 <CABPTF7XAb7ExE-7qSsKgSw3K0hfsNPzXuupQ1aJ8zXOgZ4tPNw@mail.gmail.com>
Subject: Re: t/035_standby_logical_decoding.pl might fail on attempt to read
 wrong timeline
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 Alexander Lakhin <exclusion@gmail.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CABPTF7XAb7ExE-7qSsKgSw3K0hfsNPzXuupQ1aJ8zXOgZ4tPNw%40mail.gmail.com>
Precedence: bulk

On Tue, Jun 9, 2026 at 7:01=E2=80=AFPM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jun 09, 2026 at 03:49:50PM +0800, Xuneng Zhou wrote:
> > On Mon, Jun 8, 2026 at 10:34=E2=80=AFPM Xuneng Zhou <xunengzhou@gmail.c=
om> wrote:
> > >
> > > On Mon, Jun 8, 2026 at 10:22=E2=80=AFPM Bertrand Drouvot
> > > <bertranddrouvot.pg@gmail.com> wrote:
> >
> > I've readed through the patch set. They look good overall.
>
> Thanks for the review!
>
> > Here're
> > some comments on them:
> >
> > 1) In the commit messages and comments for all four patches, the
> > reason why the target WAL segment cannot be found on the old timeline
> > is described as follows:
> >
> > "old timeline WAL segments have already been removed or
> > recycled by RemoveNonParentXlogFiles() in CleanupAfterArchiveRecovery()=
."
> >
> > Is mentioning the 'remove' case only a bit narrow?
> >
> > The timeline-selection comment says this explicitly:
> > "there's no guarantee the old segment will still exist. It may have bee=
n
> > deleted or renamed with a .partial suffix"
> >
> > How about phrasing it like:
> > old timeline WAL files may have been removed, recycled, or renamed to .=
partial.
> >
> > After running the reproducer provided by Hayato-san, the standby=E2=80=
=99s
> > pg_wal directory looked like this following the failure:
> > 000000010000000000000003.partial
> > 00000002.history
> > 000000020000000000000003
> > 000000020000000000000004
> >
> > So in this repro, the requested file:
> >
> > 000000010000000000000003
> >
> > was not unlinked as a regular "removed" file. It had been renamed to:
> >
> > 000000010000000000000003.partial
> >
> > but the log says this explicitly:
> > ERROR: requested WAL segment 000000010000000000000003 has already been =
removed
> >
> > It appears inconsistent to me...
>
> I'm not sure. The error message says "has already been removed" and the c=
ommit
> messages and comments  says"removed or recycled": those are consistent wi=
th the
> error message. We're describing the symptom from the walsender's perspect=
ive,
> not the exact file operation that caused it.
> > 2) Injection points in tests 0002 and 0004
> >
> > It does not prove this:
> > walsender has reached logical_read_xlog_page() while startup is paused
> >
> > 3) Stricter synchronization point in both tests
> > Both tests use this condition "active_pid IS NOT NULL" for
> > synchronization at the walsender side. However, it only proves that
> > pg_recvlogical has connected walsender has acquired the logical slot,
> > not necessarily the walsender is paused after acquiring the slot and
> > before the promotion window is set. There are several potential states
> > for walsender in this condition:
> >
> > walsender is just after ReplicationSlotAcquire()
> > it has called XLogBeginRead()
> > it is already inside logical_read_xlog_page()
> > it already opened the WAL segment
> > it already failed or succeeded
> >
> > The test cannot distinguish those states reliably.
> >
> > So we may still need another injection point for synchronization at
> > the walsender side
>
> I agree that with v1 the test could have been fragile. It's fixed in v2 w=
ithout
> having to add a second injection point. All we have to do is to ensure th=
at
> the decoding occurred while the startup is paused on the new injection po=
int.
>
> 0002 does that by starting the new walsender and doing the decoding while=
 the
> startup is paused
> 0004 does that by ensuring the pre-connected session triggers the decodin=
g while
> the startup is paused

That should work, and it=E2=80=99s cleverer. I was fixated on the idea that=
 we
needed to start the walsender, pause it, suspend the startup process
to enter the promotion window, and then resume the walsender. The
essential thing is just to ensure that the startup remains paused
until decoding output is observed.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.