Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: pg_stat_replication.*_lag sometimes shows NULL during active
 replication
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAHGQGwGPg20Rw2PydBDXiKHgn55s--E14-qRy7t3M+DHreCJww@mail.gmail.com>
Date: Fri, 13 Mar 2026 10:15:46 +0800
Cc: Shinya Kato <shinya11.kato@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <A65CF6F6-580B-4BF7-A0DB-FFE39E780343@gmail.com>
References: 
 <CAOzEurTzcUrEzrH97DD7+Yz=HGPU81kzWQonKZvqBwYhx2G9_A@mail.gmail.com>
 <CAHGQGwE=kyQ+YnGPn8zpZ959+3ywg8OR_Nu__uXxxuE0E+Y_Zg@mail.gmail.com>
 <CAOzEurRGiGE2Dfe+ySpb=+93ku=7ZC6RgAbHtLC6Xsq3g2XexA@mail.gmail.com>
 <CAHGQGwH2h_R7FWPvEs3+NWLwHZoj9r96tUyRKi5haqxMc6FXiQ@mail.gmail.com>
 <CAOzEurQiP3uebd1GMiC1Dzf5VJwF4ZBEpJ6QYQFE6Y+rVjxqNA@mail.gmail.com>
 <CAHGQGwEmMBBAE0RG-R3_LacfT4fbB55qGE6n9O5mNwrqvbNBtw@mail.gmail.com>
 <CAOzEurRF+OWcMZpfE=NV_Wcm6CFFGOnuxC6L9WWCjOMN0_eZMQ@mail.gmail.com>
 <CAHGQGwGPg20Rw2PydBDXiKHgn55s--E14-qRy7t3M+DHreCJww@mail.gmail.com>
To: Fujii Masao <masao.fujii@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/A65CF6F6-580B-4BF7-A0DB-FFE39E780343%40gmail.com>
Precedence: bulk


> On Mar 12, 2026, at 23:27, Fujii Masao <masao.fujii@gmail.com> wrote:
>=20
> On Wed, Mar 11, 2026 at 11:39=E2=80=AFAM Shinya Kato =
<shinya11.kato@gmail.com> wrote:
>>=20
>> On Tue, Mar 10, 2026 at 10:54=E2=80=AFAM Fujii Masao =
<masao.fujii@gmail.com> wrote:
>>> Even with your latest patch, if we remove fullyAppliedLastTime, and =
set
>>> clearLagTimes to true when applyPtr =3D=3D sentPtr && noLagSamples =
&&
>>> positionsUnchanged,
>>> wouldn't the time for the lag to become NULL be almost the same as
>>> wal_receiver_status_interval?
>>>=20
>>> The documentation doesn't clearly specify how long it should take =
for
>>> the lag to become NULL, so doubling that time might be acceptable.
>>> However, if we can keep it roughly the same without much complexity,
>>> I think that would be preferable.
>>>=20
>>> Thought?
>>=20
>> Thank you for the suggestion. I tested this by removing
>> fullyAppliedLastTime, but even with synchronous replication, NULL
>> still appears. Here is why:
>>=20
>> - Reply 1 (flush notification): positions =3D X. Lag samples are
>> consumed with real values, so noLagSamples =3D false. clearLagTimes =
is
>> not set, and prevPtrs =3D X is saved.
>>=20
>> - Reply 2 (force_reply): positions =3D X again. Here, noLagSamples =3D
>> true and positionsUnchanged =3D true. Since applyPtr =3D=3D sentPtr,
>> clearLagTimes is set to true, resulting in a NULL value.
>>=20
>> Therefore, I believe fullyAppliedLastTime is still necessary to =
ensure
>> that the previous reply also contained no lag samples.
>=20
> Thanks for testing and for the clarification! You're right.
>=20
> However, if we apply this change, the time required for the lag =
information to
> be reset would effectively double. I start wondering if that's really
> acceptable, especially for back branches. Although the docs doesn't =
clearly
> specify this timing, doubling it could affect systems that monitor
> replication lag, for example. It might still be reasonable to apply
> such a change in master, though.
>=20
> On further thought, the root cause seems to be that walreceiver can =
send
> two consecutive status reply messages with identical WAL locations =
even
> when wal_receiver_status_interval has not yet elapsed. Addressing that
> behavior directly might resolve the issue you reported. I've attached =
a PoC
> patch that does this. Thought?
>=20
> Regards,
>=20
> --=20
> Fujii Masao
> <v4-0001-Avoid-sending-duplicate-WAL-locations-in-standby-.patch>

I just read v4. The approach looks good to me overall. I have a few =
comments about the naming.

This patch changes the old force reply logic to an =
applied-location-driven reply. Now a reply is sent only if the applied =
location has advanced. However, this applied-location-driven reply is =
still triggered from WalRcvForceReply(), so the function has effectively =
lost its original =E2=80=9Cforce=E2=80=9D semantics. Because of that, it =
might be better to rename WalRcvForceReply() to something like =
WalRcvRequestApplyStatusUpdate().

Then,
```
static void
XLogWalRcvSendReply(bool force, bool requestReply, bool replyApply)
```

replyApply reads like =E2=80=9Csend an apply reply=E2=80=9D, but in =
reality it indicates that the applied location should be checked to =
decide whether to send the reply. So it might be clearer to rename it to =
something like checkApplyStatus.

Lastly,
```
    sig_atomic_t reply_apply; /* used as a bool */
```

reply_apply sounds like an action of =E2=80=9Creply with apply=E2=80=9D, =
but what it actually represents is that the startup process requested an =
applied-location-driven reply. If applied location is not advanced, the =
reply won=E2=80=99t be sent. So a name like apply_update_requested might =
better reflect the meaning.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/