On Fri, Sep 11, 2026 at 6:32 AM Xuneng Zhou <[email protected]> wrote: > On Fri, Sep 11, 2026 at 5:59 AM Alexander Korotkov <[email protected]> > wrote: > > > > On Wed, Sep 9, 2026 at 8:39 AM Xuneng Zhou <[email protected]> wrote: > > > On Wed, Sep 9, 2026 at 11:38 AM Xuneng Zhou <[email protected]> wrote: > > > > > > > > On Mon, Sep 7, 2026 at 7:28 PM Alexander Korotkov > > > > <[email protected]> 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? > > > > I think 0001-0003 are ready. I'm going to push them if no objections. > > And I'll review of 0004 in the next couple days. > > Thanks. Before the move, would you mind change a line of comment for > the restriction in v3-0001 from > > + * WAIT FOR LSN does not register its dependency on WAL progress as a > > to > > * However, we do not register our dependency on WAL progress as a > > I did not adjust it properly in the last try.
Committed, including this change. ------ Regards, Alexander Korotkov Supabase
