Re: Implement waiting for wal lsn replay: reloaded

On Wed, Sep 9, 2026 at 11:38 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>
> 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.

While reading the 0001 patch, I noticed that the current note section is
kinda not easy to follow after adding more restrictions and caveats over
time. It has eight paragraphs after the 0001 patch. The first four is a
discussion of execution restrictions, but then the text returns to
explaining every mode before moving into promotion, timelines, and recovery
conflicts. That interruption makes the section feel a bit scattered. Using
subsections like caveats, restrictions, edge cases might help, but their
boundaries seem not very clear to me. Therefore, I am considering grouping
and reorganizing this section by sub-topics. The section after the 0004
patch will look like this:

refentry: WAIT FOR
└── refsect1: Notes
├── refsect2: Execution Restrictions
│ ├── itemizedlist
│ │ ├── listitem → para: execution and snapshot restrictions
│ │ └── listitem → para: held-lock restriction and exception
│ ├── para: recommended statement order
│ ├── para: deadlock rationale
│ └── para: why write/flush modes are also restricted
└── refsect2: Recovery and Timelines
├── para: local promotion
├── para: upstream timeline changes
└── para: recovery-conflict interruptions

Is this sensible?

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