MIME-Version: 1.0
References: 
 <CABPTF7Xs-64GQNjmbimZNhj2YSKbBny+evz6=cp3X2fkJS+vMQ@mail.gmail.com>
 <CABPTF7Ub=w7CRxi3sNv8oMGMh4hCqUTohuiTuP9Y1DpxRuFtRQ@mail.gmail.com>
 <CAPpHfduJKv9-R2HcpyX9RNgteLL0M1MPS1No1WLnTsegsbG4MQ@mail.gmail.com>
 <CABPTF7WWxgAAr5fT9TFciU+PzeRpC3Dp7SO60AV9XWx561TNKA@mail.gmail.com>
 <CABPTF7X0n=R50z2fBpj3EbYYz04Ab0-DHJa+JfoAEny62QmUdg@mail.gmail.com>
 <CABPTF7U+SUnJX_woQYGe==R9Oz+-V6X0VO2stBLPGfJmH_LEhw@mail.gmail.com>
 <CABPTF7UcuVD0L-X=jZFfeygjPaZWWkVRwtWOaJw2tcXbEN2xsA@mail.gmail.com>
 <CABPTF7Wdq6KbvC3EhLX3Pz=ODCCPEX7qVQ+E=cokkB91an2E-A@mail.gmail.com>
 <CAPpHfdv_BS7csGyg_=pPanRQM9Sf6_wBWNGdVzJRAv0U4eH9cg@mail.gmail.com>
 <CAPpHfds7oSCbZqob7ytT_Lso8fv-NW8LnedUTE4Krde+3rkJeA@mail.gmail.com>
 <CABPTF7WiDtWCR82geyaaaCRV9UiPR5YUHRNDysD_7Ltr1ymfug@mail.gmail.com>
 <CABPTF7Um7oRKBYmek_3gfbaMg5DLzHZmAX-GEacHASfD34xo5g@mail.gmail.com>
 <CABPTF7V-E_e3kQ2vtwUz6Jy7u-8_YeUT0SDoAbu7EKPgNp=ndA@mail.gmail.com>
 <CAPpHfdtNiSqQCu+YtTYcc+K4q9FwtZuAtQ5Qs+KoaZZM4QyYTA@mail.gmail.com>
 <63f6abc9-c0ae-465d-a4e6-667eca6ea008@gmail.com>
 <CABPTF7V=q+OCSHLMV0v78UU=pz6b7rC81vJ5+XKnVkfAebyEwA@mail.gmail.com>
 <CABPTF7X61iuyZT3K3rZssi9qyBsfo+cGs4vWBZ6KexHi1L6hMA@mail.gmail.com>
In-Reply-To: 
 <CABPTF7X61iuyZT3K3rZssi9qyBsfo+cGs4vWBZ6KexHi1L6hMA@mail.gmail.com>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Wed, 20 May 2026 15:16:45 +0300
Message-ID: 
 <CAPpHfdtQGQpUXoqWpRghhoG_-PbUtxNTgza=am4-HHa=EmXAVQ@mail.gmail.com>
Subject: Re: Implement waiting for wal lsn replay: reloaded
To: Xuneng Zhou <xunengzhou@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>,
 Heikki Linnakangas <hlinnaka@iki.fi>,
	Peter Eisentraut <peter@eisentraut.org>, Andres Freund <andres@anarazel.de>,
	Thomas Munro <thomas.munro@gmail.com>,
 =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
	Chao Li <li.evan.chao@gmail.com>,
 pgsql-hackers <pgsql-hackers@lists.postgresql.org>,
	Michael Paquier <michael@paquier.xyz>, jian he <jian.universality@gmail.com>,
	Tomas Vondra <tomas@vondra.me>, Yura Sokolov <y.sokolov@postgrespro.ru>
Content-Type: multipart/mixed; boundary="000000000000dd3ce506523ec55d"
Archived-At: 
 <https://www.postgresql.org/message-id/CAPpHfdtQGQpUXoqWpRghhoG_-PbUtxNTgza%3Dam4-HHa%3DEmXAVQ%40mail.gmail.com>
Precedence: bulk

--000000000000dd3ce506523ec55d
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi, Xuneng!

On Wed, May 20, 2026 at 8:18=E2=80=AFAM Xuneng Zhou <xunengzhou@gmail.com> =
wrote:
> On Tue, May 19, 2026 at 8:30=E2=80=AFPM Xuneng Zhou <xunengzhou@gmail.com=
> wrote:
> >
> > Hi,
> >
> > On Tue, May 19, 2026 at 1:00=E2=80=AFPM Alexander Lakhin <exclusion@gma=
il.com> wrote:
> > >
> > > Hello Alexander and Xuneng,
> > >
> > > 06.04.2026 22:49, Alexander Korotkov wrote:
> > >
> > > Thank you, I've pushed your version of patchset.  I made two minor
> > > corrections for patch #2: mention default mode value in the header
> > > comment, and fallback to polling on has_wal_read_bug sparc64+ext4 bug=
.
> > >
> > >
> > > I discovered a new test failure, that is apparently caused by new
> > > wait_for_catchup() implementation [1]:
> > > [06:20:23.110](1.069s) not ok 8 - check that the slot state changes t=
o "extended"
> > > [06:20:23.110](0.001s) #   Failed test 'check that the slot state cha=
nges to "extended"'
> > > #   at /Users/ec2-user/bf/goldfish/HEAD/pgsql/src/test/recovery/t/019=
_replslot_limit.pl line 140.
> > > [06:20:23.111](0.000s) #          got: 'unreserved'
> > > #     expected: 'extended'
> > > [06:20:23.231](0.120s) not ok 9 - check that the slot state changes t=
o "unreserved"
> > > [06:20:23.231](0.000s) #   Failed test 'check that the slot state cha=
nges to "unreserved"'
> > > #   at /Users/ec2-user/bf/goldfish/HEAD/pgsql/src/test/recovery/t/019=
_replslot_limit.pl line 152.
> > > [06:20:23.231](0.000s) #          got: 'lost|'
> > > #     expected: 'unreserved|t'
> > >
> > > I've managed to reproduce such failures with:
> > > diff --git a/src/backend/replication/walreceiver.c b/src/backend/repl=
ication/walreceiver.c
> > > index 07eac07b9ce..493ce92674e 100644
> > > --- a/src/backend/replication/walreceiver.c
> > > +++ b/src/backend/replication/walreceiver.c
> > > @@ -1143,2 +1143,3 @@ XLogWalRcvSendReply(bool force, bool requestRep=
ly, bool checkApply)
> > >
> > > +pg_usleep(10000);
> > >      /* Get current timestamp. */
> > > diff --git a/src/backend/replication/walsender.c b/src/backend/replic=
ation/walsender.c
> > > index 04aa770d981..19cda3a6b51 100644
> > > --- a/src/backend/replication/walsender.c
> > > +++ b/src/backend/replication/walsender.c
> > > @@ -2521,2 +2521,3 @@ ProcessStandbyReplyMessage(void)
> > >
> > > +pg_usleep(100000);
> > >      /* the caller already consumed the msgtype byte */
> > >
> > > Concretely, a loop:
> > > for i in {1..100}; do echo "ITERATION $i"; PROVE_TESTS=3D"t/019*" mak=
e -s check -C src/test/recovery/ || break; done
> > > failed for me on iterations 2, 1, 7:
> > > ITERATION 7
> > > # +++ tap check in src/test/recovery +++
> > > t/019_replslot_limit.pl .. 8/?
> > > #   Failed test 'check that the slot state changes to "extended"'
> > > #   at t/019_replslot_limit.pl line 140.
> > > #          got: 'unreserved'
> > > #     expected: 'extended'
> > > t/019_replslot_limit.pl .. 26/? # Looks like you failed 1 test of 26.
> > > t/019_replslot_limit.pl .. Dubious, test returned 1 (wstat 256, 0x100=
)
> > > Failed 1/26 subtests
> > >
> > > With "WAIT FOR LSN" in wait_for_catchup() disabled, 100 iterations
> > > passed.
> > >
> > > Having extra logging added, I could see the key difference.
> > > Failed run:
> > > 2026-05-19 22:01:37.968 EEST client backend[3632148] 019_replslot_lim=
it.pl LOG:  !!!GetWALAvailability| targetLSN: 0/016C0000, targetSeg: 22, ol=
destSlotSeg: 23, oldestSegMaxWalSize: 24, oldestSeg: 22
> > > 2026-05-19 22:01:37.968 EEST client backend[3632148] 019_replslot_lim=
it.pl STATEMENT:  SELECT wal_status FROM pg_replication_slots WHERE slot_na=
me =3D 'rep1'
> > > vs
> > > Successful run:
> > > 2026-05-19 22:04:18.102 EEST client backend[3633761] 019_replslot_lim=
it.pl LOG:  !!!GetWALAvailability| targetLSN: 0/01700000, targetSeg: 23, ol=
destSlotSeg: 23, oldestSegMaxWalSize: 24, oldestSeg: 23
> > > 2026-05-19 22:04:18.102 EEST client backend[3633761] 019_replslot_lim=
it.pl STATEMENT:  SELECT wal_status FROM pg_replication_slots WHERE slot_na=
me =3D 'rep1'
> > >
> > > That is, with WAIT FOR LSN, primary in this test may advance
> > > slot->data.restart_lsn to the expected position after wait_for_catchu=
p()
> > > returns.
> > >
> > > [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=3Dgoldfis=
h&dt=3D2026-05-13%2006%3A15%3A03
> >
> > Thanks for reporting this issue.
> >
> > I think this is related to the semantic change made earlier:
> > wait_for_catchup() now returns once the standby itself has reached the
> > target LSN, rather than waiting until the primary observes that
> > position via pg_stat_replication.
> >
> > As a result, the primary may not yet have processed the standby
> > feedback needed to advance the slot's restart_lsn when
> > wait_for_catchup() returns.
> >
> > Actually, I was aware of this semantic change and previously thought
> > it might be harmless. But this failure appears to disprove that. I'll
> > prepare a patch to fix this shortly.
>
> After some consideration, 019_replslot_limit.pl appears to the better
> place to place the fix rather than by restoring the old primary-side
> polling barrier in wait_for_catchup().
>
> The new wait_for_catchup() behavior is closer to its natural
> semantics: for replay/write/flush modes, it waits until the standby
> itself has reached the requested LSN. The old implementation used
> pg_stat_replication on the primary, which also implied that the
> primary had received and processed standby feedback. That was a useful
> side effect for this test, but it is not required by most callers.
>
> 019_replslot_limit.pl is different because it checks primary-side slot
> state. For a physical slot, restart_lsn advances only after the
> primary's walsender processes standby feedback. So the test needs an
> extra condition beyond ordinary standby catchup.
>
> The patch makes that dependency explicit: wait for the standby to
> replay the target LSN, then wait for the slot's restart_lsn on the
> primary to pass the same LSN. This keeps wait_for_catchup() focused on
> standby catchup while making the slot-specific synchronization visible
> in the test that needs it.

I agree with you.  But do we actually need a
wait_for_standby_and_slot_catchup() wrapper.  I think we can call
$node->wait_for_slot_catchup() directly and simplify the fix.  Check
the attached patch.

------
Regards,
Alexander Korotkov
Supabase

--000000000000dd3ce506523ec55d
Content-Type: application/octet-stream;
	name="v2-0001-Stabilize-019_replslot_limit.pl-after-wait_for_ca.patch"
Content-Disposition: attachment;
	filename="v2-0001-Stabilize-019_replslot_limit.pl-after-wait_for_ca.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mpe10l2s0>
X-Attachment-Id: f_mpe10l2s0

RnJvbSA2MjMzNTU0NWU2OGFjMTQzMzU0NjdkM2NmZTVmNGIzZmRkMmQ1ZDI1IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBbGV4YW5kZXIgS29yb3Rrb3YgPGFrb3JvdGtvdkBwb3N0Z3Jl
c3FsLm9yZz4KRGF0ZTogV2VkLCAyMCBNYXkgMjAyNiAxNToxMjozNyArMDMwMApTdWJqZWN0OiBb
UEFUQ0ggdjJdIFN0YWJpbGl6ZSAwMTlfcmVwbHNsb3RfbGltaXQucGwgYWZ0ZXIgd2FpdF9mb3Jf
Y2F0Y2h1cCgpCiBzZW1hbnRpYyBjaGFuZ2UKCndhaXRfZm9yX2NhdGNodXAoKSBub3cgcmV0dXJu
cyBhcyBzb29uIGFzIHRoZSBzdGFuZGJ5IGhhcyByZXBsYXllZCB0aGUKdGFyZ2V0IExTTiBsb2Nh
bGx5LCByYXRoZXIgdGhhbiB3YWl0aW5nIHVudGlsIHRoZSBwcmltYXJ5IG9ic2VydmVzIHRoYXQK
cG9zaXRpb24gdmlhIHBnX3N0YXRfcmVwbGljYXRpb24uICAwMTlfcmVwbHNsb3RfbGltaXQucGws
IGhvd2V2ZXIsCmNoZWNrcyBwcmltYXJ5LXNpZGUgcGdfcmVwbGljYXRpb25fc2xvdHMgc3RhdGUs
IHdoaWNoIGRlcGVuZHMgb24gdGhlCnNsb3QncyByZXN0YXJ0X2xzbiAtLSBhbmQgcmVzdGFydF9s
c24gYWR2YW5jZXMgb25seSBhZnRlciB0aGUgcHJpbWFyeSdzCndhbHNlbmRlciBwcm9jZXNzZXMg
YSBzdGFuZGJ5IHJlcGx5LiAgVGhlIHByZXZpb3VzIHBvbGxpbmcKd2FpdF9mb3JfY2F0Y2h1cCgp
IGltcGxpY2l0bHkgd2FpdGVkIGZvciB0aGF0IHJvdW5kIHRyaXA7IHRoZQpXQUlUIEZPUiBMU04t
YmFzZWQgb25lIGRvZXMgbm90LCBzbyB0aGUgc3VidGVzdHMgImNoZWNrIHRoYXQgdGhlIHNsb3QK
c3RhdGUgY2hhbmdlcyB0byAnZXh0ZW5kZWQnIC8gJ3VucmVzZXJ2ZWQnIiBiZWNvbWUgZmxhcHB5
IChyZXByb2R1Y2libGUKd2l0aCBzbWFsbCBhcnRpZmljaWFsIGRlbGF5cyBpbiBYTG9nV2FsUmN2
U2VuZFJlcGx5IC8KUHJvY2Vzc1N0YW5kYnlSZXBseU1lc3NhZ2UpLgoKUmVwbGFjZSBlYWNoIHdh
aXRfZm9yX2NhdGNodXAoKSBpbiB0aGlzIHRlc3Qgd2l0aAp3YWl0X2Zvcl9zbG90X2NhdGNodXAo
J3JlcDEnLCAncmVzdGFydCcsIHByaW1hcnktPmxzbignd3JpdGUnKSkuCnJlc3RhcnRfbHNuIGNh
bm5vdCBtb3ZlIGFoZWFkIG9mIHRoZSBzdGFuZGJ5J3MgcmVwbGF5ZWQgcG9zaXRpb24sIHNvCnRo
aXMgc2luZ2xlIHdhaXQgdHJhbnNpdGl2ZWx5IGNvdmVycyBib3RoIHRoZSBzdGFuZGJ5IHJlcGxh
eSBhbmQgdGhlCnByaW1hcnkncyBvYnNlcnZhdGlvbiBvZiBpdCwgd2hpY2ggaXMgZXhhY3RseSB0
aGUgcHJlY29uZGl0aW9uIHRoZQpzbG90LXN0YXRlIGFzc2VydGlvbnMgcmVxdWlyZS4KClJlcG9y
dGVkLWJ5OiBBbGV4YW5kZXIgTGFraGluIDxleGNsdXNpb25AZ21haWwuY29tPgpEaXNjdXNzaW9u
OiBodHRwczovL3Bvc3Rnci5lcy9tLzYzZjZhYmM5LWMwYWUtNDY1ZC1hNGU2LTY2N2VjYTZlYTAw
OEBnbWFpbC5jb20KQXV0aG9yOiBYdW5lbmcgWmhvdSA8eHVuZW5nemhvdUBnbWFpbC5jb20+CkF1
dGhvcjogQWxleGFuZGVyIEtvcm90a292IDxhZWtvcm90a292QGdtYWlsLmNvbT4KLS0tCiBzcmMv
dGVzdC9yZWNvdmVyeS90LzAxOV9yZXBsc2xvdF9saW1pdC5wbCB8IDIwICsrKysrKysrKysrKysr
LS0tLS0tCiAxIGZpbGUgY2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygrKSwgNiBkZWxldGlvbnMoLSkK
CmRpZmYgLS1naXQgYS9zcmMvdGVzdC9yZWNvdmVyeS90LzAxOV9yZXBsc2xvdF9saW1pdC5wbCBi
L3NyYy90ZXN0L3JlY292ZXJ5L3QvMDE5X3JlcGxzbG90X2xpbWl0LnBsCmluZGV4IDdiMjUzZTY0
ZDljLi40NzJhYTA3NTg3ZiAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvcmVjb3ZlcnkvdC8wMTlfcmVw
bHNsb3RfbGltaXQucGwKKysrIGIvc3JjL3Rlc3QvcmVjb3ZlcnkvdC8wMTlfcmVwbHNsb3RfbGlt
aXQucGwKQEAgLTQ0LDggKzQ0LDEyIEBAICRub2RlX3N0YW5kYnktPmFwcGVuZF9jb25mKCdwb3N0
Z3Jlc3FsLmNvbmYnLCAicHJpbWFyeV9zbG90X25hbWUgPSAncmVwMSciKTsKIAogJG5vZGVfc3Rh
bmRieS0+c3RhcnQ7CiAKLSMgV2FpdCB1bnRpbCBzdGFuZGJ5IGhhcyByZXBsYXllZCBlbm91Z2gg
ZGF0YQotJG5vZGVfcHJpbWFyeS0+d2FpdF9mb3JfY2F0Y2h1cCgkbm9kZV9zdGFuZGJ5KTsKKyMg
V2FpdCB1bnRpbCB0aGUgcHJpbWFyeSBoYXMgcHJvY2Vzc2VkIHN0YW5kYnkgZmVlZGJhY2sgYW5k
IGFkdmFuY2VkCisjIHRoZSBzbG90J3MgcmVzdGFydF9sc24uICByZXN0YXJ0X2xzbiBtb3ZlcyBv
bmx5IGFmdGVyIHRoZSBzdGFuZGJ5J3MKKyMgcmVwbHkgcmVhY2hlcyB0aGUgd2Fsc2VuZGVyLCBz
byB0aGlzIHRyYW5zaXRpdmVseSBndWFyYW50ZWVzIHRoYXQKKyMgdGhlIHN0YW5kYnkgaXRzZWxm
IGhhcyByZXBsYXllZCBwYXN0IHRoZSB0YXJnZXQgTFNOLgorJG5vZGVfcHJpbWFyeS0+d2FpdF9m
b3Jfc2xvdF9jYXRjaHVwKCdyZXAxJywgJ3Jlc3RhcnQnLAorCSRub2RlX3ByaW1hcnktPmxzbign
d3JpdGUnKSk7CiAKICMgU3RvcCBzdGFuZGJ5CiAkbm9kZV9zdGFuZGJ5LT5zdG9wOwpAQCAtNzks
NyArODMsOCBAQCBpcygkcmVzdWx0LCAicmVzZXJ2ZWR8dCIsICdjaGVjayB0aGF0IHNsb3QgaXMg
d29ya2luZycpOwogIyBUaGUgc3RhbmRieSBjYW4gcmVjb25uZWN0IHRvIHByaW1hcnkKICRub2Rl
X3N0YW5kYnktPnN0YXJ0OwogCi0kbm9kZV9wcmltYXJ5LT53YWl0X2Zvcl9jYXRjaHVwKCRub2Rl
X3N0YW5kYnkpOworJG5vZGVfcHJpbWFyeS0+d2FpdF9mb3Jfc2xvdF9jYXRjaHVwKCdyZXAxJywg
J3Jlc3RhcnQnLAorCSRub2RlX3ByaW1hcnktPmxzbignd3JpdGUnKSk7CiAKICRub2RlX3N0YW5k
YnktPnN0b3A7CiAKQEAgLTEwOSw3ICsxMTQsOCBAQCBpcygkcmVzdWx0LCAicmVzZXJ2ZWQiLAog
CiAjIFRoZSBzdGFuZGJ5IGNhbiByZWNvbm5lY3QgdG8gcHJpbWFyeQogJG5vZGVfc3RhbmRieS0+
c3RhcnQ7Ci0kbm9kZV9wcmltYXJ5LT53YWl0X2Zvcl9jYXRjaHVwKCRub2RlX3N0YW5kYnkpOwor
JG5vZGVfcHJpbWFyeS0+d2FpdF9mb3Jfc2xvdF9jYXRjaHVwKCdyZXAxJywgJ3Jlc3RhcnQnLAor
CSRub2RlX3ByaW1hcnktPmxzbignd3JpdGUnKSk7CiAkbm9kZV9zdGFuZGJ5LT5zdG9wOwogCiAj
IHdhbF9rZWVwX3NpemUgb3ZlcnJpZGVzIG1heF9zbG90X3dhbF9rZWVwX3NpemUKQEAgLTEyOCw3
ICsxMzQsOCBAQCAkcmVzdWx0ID0gJG5vZGVfcHJpbWFyeS0+c2FmZV9wc3FsKCdwb3N0Z3Jlcycs
CiAKICMgVGhlIHN0YW5kYnkgY2FuIHJlY29ubmVjdCB0byBwcmltYXJ5CiAkbm9kZV9zdGFuZGJ5
LT5zdGFydDsKLSRub2RlX3ByaW1hcnktPndhaXRfZm9yX2NhdGNodXAoJG5vZGVfc3RhbmRieSk7
Ciskbm9kZV9wcmltYXJ5LT53YWl0X2Zvcl9zbG90X2NhdGNodXAoJ3JlcDEnLCAncmVzdGFydCcs
CisJJG5vZGVfcHJpbWFyeS0+bHNuKCd3cml0ZScpKTsKICRub2RlX3N0YW5kYnktPnN0b3A7CiAK
ICMgQWR2YW5jZSBXQUwgYWdhaW4gd2l0aG91dCBjaGVja3BvaW50LCByZWR1Y2luZyByZW1haW4g
YnkgNiBNQi4KQEAgLTE1NSw3ICsxNjIsOCBAQCBpcygkcmVzdWx0LCAidW5yZXNlcnZlZHx0IiwK
ICMgVGhlIHN0YW5kYnkgc3RpbGwgY2FuIGNvbm5lY3QgdG8gcHJpbWFyeSBiZWZvcmUgYSBjaGVj
a3BvaW50CiAkbm9kZV9zdGFuZGJ5LT5zdGFydDsKIAotJG5vZGVfcHJpbWFyeS0+d2FpdF9mb3Jf
Y2F0Y2h1cCgkbm9kZV9zdGFuZGJ5KTsKKyRub2RlX3ByaW1hcnktPndhaXRfZm9yX3Nsb3RfY2F0
Y2h1cCgncmVwMScsICdyZXN0YXJ0JywKKwkkbm9kZV9wcmltYXJ5LT5sc24oJ3dyaXRlJykpOwog
CiAkbm9kZV9zdGFuZGJ5LT5zdG9wOwogCi0tIAoyLjM5LjUgKEFwcGxlIEdpdC0xNTQpCgo=
--000000000000dd3ce506523ec55d--