t/035_standby_logical_decoding.pl might fail on attempt to read wrong timeline

Hi Alexander,

On Sat, Jun 06, 2026 at 12:00:00PM +0300, Alexander Lakhin wrote:

Hello hackers,

That is, walsender requested WAL segment for timeline 1, while in a
successful run, it reads WAL for timeline 2.

I've managed to reproduce this failure with:

Thanks for the report and the repro!

As far as I can see, the timeline is chosen in logical_read_xlog_page()
depending on the recovery state:
        am_cascading_walsender = RecoveryInProgress();

        if (am_cascading_walsender)
                GetXLogReplayRecPtr(&currTLI);
        else
                currTLI = GetWALInsertionTimeLine();

Yeah, it looks like there is a race condition here. I think we should check if
the insertion timeline has already been set (like the walsummarizer is doing).

I'll work on a fix early next week.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Hi Bertrand,

On Sat, Jun 6, 2026 at 7:07 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi Alexander,

On Sat, Jun 06, 2026 at 12:00:00PM +0300, Alexander Lakhin wrote:

Hello hackers,

That is, walsender requested WAL segment for timeline 1, while in a
successful run, it reads WAL for timeline 2.

I've managed to reproduce this failure with:

Thanks for the report and the repro!

As far as I can see, the timeline is chosen in logical_read_xlog_page()
depending on the recovery state:
am_cascading_walsender = RecoveryInProgress();

if (am_cascading_walsender)
GetXLogReplayRecPtr(&currTLI);
else
currTLI = GetWALInsertionTimeLine();

Yeah, it looks like there is a race condition here. I think we should check if
the insertion timeline has already been set (like the walsummarizer is doing).

I'll work on a fix early next week.

This looks like the right direction to fix. We may want to apply
similar logic to read_local_xlog_page_guts as well. Although the
failure is reported in walsender, SQL logical decoding uses the local
WAL reader and has the same recovery/TLI pattern.

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

Hi Alexander, Bertrand, Xuneng,

Thanks for seeing the failure. Our team also recognized but could not find the reason.

Yeah, it looks like there is a race condition here. I think we should check if
the insertion timeline has already been set (like the walsummarizer is doing).

Sorry for stupid question; I tried to reproduce the failure but could not, see attached.

IIUC, the issue can happen if the walsender must read the WAL record generated
after the promotion but the timeline could not be updated.

However, I think logical_read_xlog_page() is called after the new WAL records
are generated, i.e., am_cascading_walsender has already been false at that time.
So not sure where is the race?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi Kuroda-san,

On Mon, Jun 8, 2026 at 12:25 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Hi Alexander, Bertrand, Xuneng,

Thanks for seeing the failure. Our team also recognized but could not find the reason.

Yeah, it looks like there is a race condition here. I think we should check if
the insertion timeline has already been set (like the walsummarizer is doing).

Sorry for stupid question; I tried to reproduce the failure but could not, see attached.

IIUC, the issue can happen if the walsender must read the WAL record generated
after the promotion but the timeline could not be updated.

I think the race is that the logical walsender is trying to read the
pre-promotion wal from the slot's restart_lsn.

1) In 035_standby_logical_decoding.pl, the active slot /
pg_recvlogical process is started before:
INSERT rows 1..4 # before promotion
promote standby
INSERT rows 5..7 # after promotion

2) Alexander’s reproducer sleeps immediately after:
ReplicationSlotAcquire(cmd->slotname, true, true);
pg_usleep(200000);

So with the delay:
slot is active but walsender has not yet create decoding context and
called xlogbeginread();

3) Then the test continues and promotes the standby while the
walsender still sleeping, therefore promotion could happen before the
walsender starts its actual wal read.

4) The logical walsender is trying to read the pre-promotion wal from
the slot's restart_lsn. The failing log implies this:
Streaming transactions committing after 0/06487A20,
reading WAL from 0/064879E8.

This line is emitted from:
errdetail("Streaming transactions committing after %X/%08X, reading
WAL from %X/%08X.",
slot->data.confirmed_flush,
slot->data.restart_lsn)

Function StartLogicalReplication() initiates the xlogreader at the restart lsn:
XLogBeginRead(logical_decoding_ctx->reader,
MyReplicationSlot->data.restart_lsn);

Because the slot was created and acquired before promotion, its
restart_lsn is from the standby-era state which points to wal required
for decoding the pre-promotion stream. In the test, the first expected
output is rows 1..4, inserted before promotion.

The failing output was empty:
got: ''
expected:
BEGIN
rows 1..4
COMMIT
BEGIN
rows 5..7
COMMIT

Empty output suggests that the walsender died before emitting even the
first pre-promotion transaction.

5) The walsender try to read wal but failed
Failed run:
LOG: !!!WalSndSegmentOpen| nextSegNo: 6,
path: pg_wal/000000010000000000000006
ERROR: requested WAL segment
000000010000000000000006 has already been removed

Passed run
LOG: !!!WalSndSegmentOpen| nextSegNo: 6,
path: pg_wal/000000020000000000000006

As the log suggested, the walsender of the successful run readed the
wal from timeline 2 but failed to do so for timeline1.
This comment in xlogutils.is almost exactly the issue:

We care about timelines ... reading xlog generated prior to a promotion
...
The server copied the segment to the new timeline ... there's no guarantee
the old segment will still exist. It may have been deleted or renamed with
a .partial suffix

So the need for tli1 does not always imply that the wal was generated
after promotion. It could be bytes from before the switchpoint, but
they now be available through the new tli segment file.

6) XLogReadDetermineTimeline() needs the current system-wide TLI.

The current code in does roughly:

am_cascading_walsender = RecoveryInProgress();

if (am_cascading_walsender)
GetXLogReplayRecPtr(&currTLI);
else
currTLI = GetWALInsertionTimeLine();

During promotion, we set the wal insertion timeline before marking
recovery as fully done. So there is a window where:

- InsertTimeLineID is already the new timeline, e.g. tli 2.
- RecoveryInProgress() can still return true in another backend.
- A logical walsender that started before promotion can wake up and
choose its read timeline using the old recovery-state test.

In the race, replay tli 1 is chosen even though the promotion has
created the new tli 2 wal segment copy. Then WalSndSegmentOpen() tries
to open 000000010000000000000006, which may already be gone or
renamed, while the correct file is 000000020000000000000006.

7) Bertrand's proposal to fix this
Recognizing the intermediate promotion state -- if recovery still
shows progress but the insertion tli is already advanced, use that
insertion tli for the timeline to be chosen.

However, I think logical_read_xlog_page() is called after the new WAL records
are generated, i.e., am_cascading_walsender has already been false at that time.
So not sure where is the race?

This function is not only called after new post-promotion records are
generated. When START_REPLICATION begins, CreateDecodingContext() /
XLogBeginRead() starts from the slot's restart_lsn, which can be well
before promotion.

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

Hi,

On Mon, Jun 08, 2026 at 04:25:45AM +0000, Hayato Kuroda (Fujitsu) wrote:

Hi Alexander, Bertrand, Xuneng,

Thanks for seeing the failure. Our team also recognized but could not find the reason.

Yeah, it looks like there is a race condition here. I think we should check if
the insertion timeline has already been set (like the walsummarizer is doing).

IIUC, the issue can happen if the walsender must read the WAL record generated
after the promotion but the timeline could not be updated.

However, I think logical_read_xlog_page() is called after the new WAL records
are generated, i.e., am_cascading_walsender has already been false at that time.
So not sure where is the race?

I ended up with this conclusion:

During promotion, there is a window where RecoveryInProgress() still
returns true but old timeline WAL segments have already been removed or
recycled by RemoveNonParentXlogFiles() in CleanupAfterArchiveRecovery().
This is because, in StartupXLOG(), WAL segments are cleaned up before
SharedRecoveryState transitions to RECOVERY_STATE_DONE.

If a walsender performing logical decoding calls logical_read_xlog_page()
during this window, it would get the old timeline from GetXLogReplayRecPtr(),
then attempt to open a WAL segment on that old timeline which no longer exists.

Attached:

0001: To fix this race

Fix by checking GetWALInsertionTimeLineIfSet() when RecoveryInProgress()
returns true. If InsertTimeLineID is already set (non-zero), the new timeline is
established and we use it directly, avoiding attempts to read from segments that
may have been removed.

0002: Adding a test in 035_standby_logical_decoding.pl

It makes use of a new injection point "promotion-after-wal-segment-cleanup" in
StartupXLOG(), right after CleanupAfterArchiveRecovery() removes old timeline
WAL segments but before SharedRecoveryState is set to RECOVERY_STATE_DONE.

The test fails without the fix in 0001 so it also somehow proves that the
diagnostic is right.

0003: Apply the same timeline fix to read_local_xlog_page_guts()

Indeed, it could hit the same race as mentioned by Xuneng-San.

0004: Add a test for 0003

Remark:

As far as the backpatching down to 16, it looks like 0001 to 0004 could be
backpatched as they are down to 17. For 16, we may want to also introduce
GetWALInsertionTimeLineIfSet().

I can have a closer look for the backpatch once we agree on how to fix those
races on master.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Mon, Jun 8, 2026 at 4:31 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:

Hi Kuroda-san,

On Mon, Jun 8, 2026 at 12:25 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Hi Alexander, Bertrand, Xuneng,

Thanks for seeing the failure. Our team also recognized but could not find the reason.

Yeah, it looks like there is a race condition here. I think we should check if
the insertion timeline has already been set (like the walsummarizer is doing).

Sorry for stupid question; I tried to reproduce the failure but could not, see attached.

IIUC, the issue can happen if the walsender must read the WAL record generated
after the promotion but the timeline could not be updated.

I think the race is that the logical walsender is trying to read the
pre-promotion wal from the slot's restart_lsn.

1) In 035_standby_logical_decoding.pl, the active slot /
pg_recvlogical process is started before:
INSERT rows 1..4 # before promotion
promote standby
INSERT rows 5..7 # after promotion

2) Alexander’s reproducer sleeps immediately after:
ReplicationSlotAcquire(cmd->slotname, true, true);
pg_usleep(200000);

So with the delay:
slot is active but walsender has not yet create decoding context and
called xlogbeginread();

3) Then the test continues and promotes the standby while the
walsender still sleeping, therefore promotion could happen before the
walsender starts its actual wal read.

4) The logical walsender is trying to read the pre-promotion wal from
the slot's restart_lsn. The failing log implies this:
Streaming transactions committing after 0/06487A20,
reading WAL from 0/064879E8.

This line is emitted from:
errdetail("Streaming transactions committing after %X/%08X, reading
WAL from %X/%08X.",
slot->data.confirmed_flush,
slot->data.restart_lsn)

Function StartLogicalReplication() initiates the xlogreader at the restart lsn:
XLogBeginRead(logical_decoding_ctx->reader,
MyReplicationSlot->data.restart_lsn);

Because the slot was created and acquired before promotion, its
restart_lsn is from the standby-era state which points to wal required
for decoding the pre-promotion stream. In the test, the first expected
output is rows 1..4, inserted before promotion.

The failing output was empty:
got: ''
expected:
BEGIN
rows 1..4
COMMIT
BEGIN
rows 5..7
COMMIT

Empty output suggests that the walsender died before emitting even the
first pre-promotion transaction.

5) The walsender try to read wal but failed
Failed run:
LOG: !!!WalSndSegmentOpen| nextSegNo: 6,
path: pg_wal/000000010000000000000006
ERROR: requested WAL segment
000000010000000000000006 has already been removed

Passed run
LOG: !!!WalSndSegmentOpen| nextSegNo: 6,
path: pg_wal/000000020000000000000006

As the log suggested, the walsender of the successful run readed the
wal from timeline 2 but failed to do so for timeline1.
This comment in xlogutils.is almost exactly the issue:

We care about timelines ... reading xlog generated prior to a promotion
...
The server copied the segment to the new timeline ... there's no guarantee
the old segment will still exist. It may have been deleted or renamed with
a .partial suffix

So the need for tli1 does not always imply that the wal was generated
after promotion. It could be bytes from before the switchpoint, but
they now be available through the new tli segment file.

6) XLogReadDetermineTimeline() needs the current system-wide TLI.

The current code in does roughly:

am_cascading_walsender = RecoveryInProgress();

if (am_cascading_walsender)
GetXLogReplayRecPtr(&currTLI);
else
currTLI = GetWALInsertionTimeLine();

During promotion, we set the wal insertion timeline before marking
recovery as fully done. So there is a window where:

- InsertTimeLineID is already the new timeline, e.g. tli 2.
- RecoveryInProgress() can still return true in another backend.
- A logical walsender that started before promotion can wake up and
choose its read timeline using the old recovery-state test.

In the race, replay tli 1 is chosen even though the promotion has
created the new tli 2 wal segment copy. Then WalSndSegmentOpen() tries
to open 000000010000000000000006, which may already be gone or
renamed, while the correct file is 000000020000000000000006.

7) Bertrand's proposal to fix this
Recognizing the intermediate promotion state -- if recovery still
shows progress but the insertion tli is already advanced, use that
insertion tli for the timeline to be chosen.

However, I think logical_read_xlog_page() is called after the new WAL records
are generated, i.e., am_cascading_walsender has already been false at that time.
So not sure where is the race?

This function is not only called after new post-promotion records are
generated. When START_REPLICATION begins, CreateDecodingContext() /
XLogBeginRead() starts from the slot's restart_lsn, which can be well
before promotion.

I tweaked the reproducer based on the theory outlined above. The main
changes from the original reproducer are:

1) blocks at logical-walsender-after-slot-acquire in walsender.c,
before the decoding context is created and before the reader starts
from restart_lsn, matching the delay set by Alexander

2) Forces the first read to occur during promotion. It inserts rows
1..4, waits for replay, starts promotion with pg_promote(false), holds
startup at startup-logical-decoding-status-change-end-of-recovery,
then wakes the walsender.

The test failed on HEAD and passed after applying the patch by Bertrand.

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

Hi,

On Mon, Jun 08, 2026 at 09:00:17PM +0800, Xuneng Zhou wrote:

I tweaked the reproducer based on the theory outlined above. The main
changes from the original reproducer are:

1) blocks at logical-walsender-after-slot-acquire in walsender.c,
before the decoding context is created and before the reader starts
from restart_lsn, matching the delay set by Alexander

2) Forces the first read to occur during promotion. It inserts rows
1..4, waits for replay, starts promotion with pg_promote(false), holds
startup at startup-logical-decoding-status-change-end-of-recovery,
then wakes the walsender.

Yeah, so this existing startup-logical-decoding-status-change-end-of-recovery
injection point already exists in the code tree and is also called after
CleanupAfterArchiveRecovery() and before RECOVERY_STATE_DONE change in
StartupXLOG().

So this is the same window as the new injection point that was added in the new
tests in v1-0002 shared up-thread [1]/messages/by-id/aiaBtENl7tTf2MM8@bdtpg.

That said, I think I prefer the v1-0002 shared up-thread [1]/messages/by-id/aiaBtENl7tTf2MM8@bdtpg approach for the
tests as:

- the injection point name clearly describes the tested condition
- no new injection point is added in walsender.c (it pauses startup mid-promotion
and lets the walsender connect)
- the test relies on one injection point (not two)

[1]: /messages/by-id/aiaBtENl7tTf2MM8@bdtpg

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Mon, Jun 8, 2026 at 10:22 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Mon, Jun 08, 2026 at 09:00:17PM +0800, Xuneng Zhou wrote:

I tweaked the reproducer based on the theory outlined above. The main
changes from the original reproducer are:

1) blocks at logical-walsender-after-slot-acquire in walsender.c,
before the decoding context is created and before the reader starts
from restart_lsn, matching the delay set by Alexander

2) Forces the first read to occur during promotion. It inserts rows
1..4, waits for replay, starts promotion with pg_promote(false), holds
startup at startup-logical-decoding-status-change-end-of-recovery,
then wakes the walsender.

Yeah, so this existing startup-logical-decoding-status-change-end-of-recovery
injection point already exists in the code tree and is also called after
CleanupAfterArchiveRecovery() and before RECOVERY_STATE_DONE change in
StartupXLOG().

So this is the same window as the new injection point that was added in the new
tests in v1-0002 shared up-thread [1].

That said, I think I prefer the v1-0002 shared up-thread [1] approach for the
tests as:

- the injection point name clearly describes the tested condition
- no new injection point is added in walsender.c (it pauses startup mid-promotion
and lets the walsender connect)
- the test relies on one injection point (not two)

[1]: /messages/by-id/aiaBtENl7tTf2MM8@bdtpg

Thanks for the clarification. I haven't reviewed the patch set other
than applying the patch 1 yet, but I plan to do so tomorrow.

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

Dear Bertrand, Xuneng,

Thanks for sharing the reproducer and opinion. I could now understand.

While seeing, I felt the same issue can happen even for the streaming case.
Please see attached reproducer and possible fix.

Can you also include that point? Or this thread discusses only for the logical case?
If separated, I can take initiative for the streaming part, after the logical decoding
case.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi Hayato-san,

On Tue, Jun 09, 2026 at 04:04:20AM +0000, Hayato Kuroda (Fujitsu) wrote:

Dear Bertrand, Xuneng,

Thanks for sharing the reproducer and opinion. I could now understand.

While seeing, I felt the same issue can happen even for the streaming case.
Please see attached reproducer and possible fix.

I just had a quick look and I feel that's probably an issue too (good find!).
I think that your fix should use GetWALInsertionTimeLineIfSet() though (but I
only did a quick scan on your patch).

Can you also include that point? Or this thread discusses only for the logical case?
If separated, I can take initiative for the streaming part, after the logical decoding
case.

I would says it's probably better to keep this thread focusing on Alexander's issue
(logical decoding). Once this thread is fixed, then I'd suggest to open a dedicated
thread for the streaming one (mentioning this thread and probably trying to mimic
its fix as close as possible).

Also I would suggest not to post "real" patches here as they conflict with mine
in the cfbot (means the cfbot won't tell me anymore if my patches need a rebase).
Please use .txt files or use "nocfbot" instead if you feel the need.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Hi,

On Mon, Jun 08, 2026 at 08:47:48AM +0000, Bertrand Drouvot wrote:

Attached:

0001: To fix this race

Fix by checking GetWALInsertionTimeLineIfSet() when RecoveryInProgress()
returns true. If InsertTimeLineID is already set (non-zero), the new timeline is
established and we use it directly, avoiding attempts to read from segments that
may have been removed.

0002: Adding a test in 035_standby_logical_decoding.pl

It makes use of a new injection point "promotion-after-wal-segment-cleanup" in
StartupXLOG(), right after CleanupAfterArchiveRecovery() removes old timeline
WAL segments but before SharedRecoveryState is set to RECOVERY_STATE_DONE.

The test fails without the fix in 0001 so it also somehow proves that the
diagnostic is right.

0003: Apply the same timeline fix to read_local_xlog_page_guts()

Indeed, it could hit the same race as mentioned by Xuneng-San.

0004: Add a test for 0003

Re-attaching v1 patches here to have the cfbot focusing on those.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Tue, Jun 9, 2026 at 1:57 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi Hayato-san,

On Tue, Jun 09, 2026 at 04:04:20AM +0000, Hayato Kuroda (Fujitsu) wrote:

Dear Bertrand, Xuneng,

Thanks for sharing the reproducer and opinion. I could now understand.

While seeing, I felt the same issue can happen even for the streaming case.
Please see attached reproducer and possible fix.

Good catch!

I just had a quick look and I feel that's probably an issue too (good find!).
I think that your fix should use GetWALInsertionTimeLineIfSet() though (but I
only did a quick scan on your patch).

Can you also include that point? Or this thread discusses only for the logical case?
If separated, I can take initiative for the streaming part, after the logical decoding
case.

I would says it's probably better to keep this thread focusing on Alexander's issue
(logical decoding). Once this thread is fixed, then I'd suggest to open a dedicated
thread for the streaming one (mentioning this thread and probably trying to mimic
its fix as close as possible).

+1 for starting a dedicated thread. Please CC me in that thread in
case I miss it.

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

Dear Bertrand,

Thanks for re-posting. Let's focus on the logical decoding stuff.

0004: Add a test for 0003

I found a comment for the test.

```
+# Issue SQL decoding (read_local_xlog_page_guts path) on the pre-connected
+# session.
+$decode_session->query_until(
+       qr/decoding_started/, qq(
+       \\echo decoding_started
+       SELECT count(*) FROM pg_logical_slot_peek_changes('race_slot_sql', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
+));
```

Not sure, pg_logical_slot_peek_changes() can be finished without waiting anything,
right? So why do we use query_until() and check the output here?
IIUC, the command can immediately raise an ERROR if the race happened. So isn't it
enough to use safe_psql() to ensure the SQL function can work?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Mon, Jun 8, 2026 at 10:34 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:

On Mon, Jun 8, 2026 at 10:22 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Mon, Jun 08, 2026 at 09:00:17PM +0800, Xuneng Zhou wrote:

I tweaked the reproducer based on the theory outlined above. The main
changes from the original reproducer are:

1) blocks at logical-walsender-after-slot-acquire in walsender.c,
before the decoding context is created and before the reader starts
from restart_lsn, matching the delay set by Alexander

2) Forces the first read to occur during promotion. It inserts rows
1..4, waits for replay, starts promotion with pg_promote(false), holds
startup at startup-logical-decoding-status-change-end-of-recovery,
then wakes the walsender.

Yeah, so this existing startup-logical-decoding-status-change-end-of-recovery
injection point already exists in the code tree and is also called after
CleanupAfterArchiveRecovery() and before RECOVERY_STATE_DONE change in
StartupXLOG().

So this is the same window as the new injection point that was added in the new
tests in v1-0002 shared up-thread [1].

That said, I think I prefer the v1-0002 shared up-thread [1] approach for the
tests as:

- the injection point name clearly describes the tested condition
- no new injection point is added in walsender.c (it pauses startup mid-promotion
and lets the walsender connect)
- the test relies on one injection point (not two)

[1]: /messages/by-id/aiaBtENl7tTf2MM8@bdtpg

Thanks for the clarification. I haven't reviewed the patch set other
than applying the patch 1 yet, but I plan to do so tomorrow.

I've readed through the patch set. They look good overall. 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 been
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’s
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...

2) Injection points in tests 0002 and 0004

- the injection point name clearly describes the tested condition

I think this is true. It better reflects the intention than
startup-logical-decoding-status-change-end-of-recovery does. However,
I don't know whether this alone warrants a new injection point.

- no new injection point is added in walsender.c (it pauses startup mid-promotion
and lets the walsender connect)
- the test relies on one injection point (not two)

I am unsure that the injection point could replace the walsender-side
synchronization need.

The race has two moving processes:

- startup process: promotion cleanup / RECOVERY_STATE_DONE transition
- walsender process: logical_read_xlog_page() timeline choice

A startup injection point controls only this:
startup is paused in the vulnerable promotion window

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

4) Stricter result checks in test files
The surrounding 035 test is stricter than the test in 0002. It first
waits for COMMITs, then compares exact decoded output. Should we
adhere to this pattern too?

0004's \d+ check seems somewhat loose to me. Should we capture the
query result and compare it to the expected count, or select data and
compare the decoded rows?

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

Hi,

On Tue, Jun 09, 2026 at 03:49:50PM +0800, Xuneng Zhou wrote:

On Mon, Jun 8, 2026 at 10:34 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:

On Mon, Jun 8, 2026 at 10:22 PM 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 been
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’s
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 commit
messages and comments says"removed or recycled": those are consistent with the
error message. We're describing the symptom from the walsender's perspective,
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 without
having to add a second injection point. All we have to do is to ensure that
the decoding occurred while the startup is paused on the new injection point.

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 decoding while
the startup is paused

4) Stricter result checks in test files
The surrounding 035 test is stricter than the test in 0002. It first
waits for COMMITs, then compares exact decoded output. Should we
adhere to this pattern too?

That's done in v2 (and also adress Hayato-san feedback).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com