Re: Implement waiting for wal lsn replay: reloaded

On Mon, Sep 7, 2026 at 7:28 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>
> Hi, Xuneng!
> Thanks for the patches. I've revised 0001; 0002 and 0003 look correct
> to me as posted.

Thanks for updating the patches! 0001 looks much better after revisement.

> Changes I made to 0001:
>
> 1. The docs said the wait is rejected when the session holds a lock,
> but the code only rejects waits that are not already satisfied (lsn >
> GetCurrentLSNForWaitType()). Reworded, and stated explicitly that a
> wait whose target has already been reached always returns.

LGTM.

> 2. The message now names the offending lock, using DescribeLockTag().

+ 1. It can be useful.

> 3. Dropped "heavyweight" from the user-facing messages. The term is
> defined for users in monitoring.sgml, but no backend message uses it,
> and the errdetail repeated it; it now explains the cycle instead.

+1

> 4. Strengthened the justification for covering standby_write and
> standby_flush, in the comment, the docs and the commit message. What
> I think is the decisive argument was missing: a blocked startup
> process also stops creating restartpoints, and CreateRestartPoint() is
> what calls RemoveOldXlogFiles(), so pg_wal grows until the filesystem
> fills and reception stops as well. The independence streaming
> provides is bounded by free space in pg_wal, not durable for the life
> of the wait.

Thanks for pointing this out. I looked into the described scenario,
and it does look more concerning than my original report, which
strengthens extending the restriction.

> 5. Documented the recommended usage pattern: issue WAIT FOR outside a
> transaction block, or as its first statement, before anything that
> takes locks. Including the trap that a lock from an earlier statement
> is still held at READ COMMITTED even though its snapshot is gone, so a
> wait placed after such a statement is rejected even though the
> isolation check above it passes.

This seems helpful to me.

> 6. Added a test for the already-satisfied case, since the docs now
> state that rule.

I made some minor adjustments to the commit message, doc and comment
for the restriction, regarding the forming of deadlock cycle, why it
cannot be detected and breaked with current probing mechanism, and the
cases of unguaranteed independence streaming.

> For 0002, I only removed inline from
> PrimaryFlushWakeupProcessRequests(). I think it's not required for a
> function existing within one module with one caller (compiler may take
> decision to inline it by itself).

Make sense. Verified this by compiling the patched xlog.c with and
without inline, using local Apple Clang 21 and configured -O2 flags.
Both versions inlined the helper into XLogInsertRecord() and produced
a byte-for-byte identical assembly.

> I'm still considering your question 4, will return with my opinion on
> it a bit later.

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