Clarify what is protected by WaitLSNLock

Not just WaitLSNState.waitersHeap, but also WaitLSNState.procInfos and
updating of WaitLSNState.minWaitedLSN is protected by WaitLSNLock.  There
is one now documented exclusion on fast-path checking of
WaitLSNProcInfo.inHeap flag.

Discussion: https://postgr.es/m/202404030658.hhj3vfxeyhft%40alvherre.pgsql

diff --git a/src/backend/commands/waitlsn.c b/src/backend/commands/waitlsn.c
index a57b818a2d482dd633deb085c50d394a09fdbb6a..1a83c34e09f7328b05c3db9a34c06fe3af3584be 100644 (file)
--- a/src/backend/commands/waitlsn.c
+++ b/src/backend/commands/waitlsn.c
@@ -109,13 +109,13 @@ addLSNWaiter(XLogRecPtr lsn)
 {
    WaitLSNProcInfo *procInfo = &waitLSN->procInfos[MyProcNumber];
 
+   LWLockAcquire(WaitLSNLock, LW_EXCLUSIVE);
+
    Assert(!procInfo->inHeap);
 
    procInfo->procnum = MyProcNumber;
    procInfo->waitLSN = lsn;
 
-   LWLockAcquire(WaitLSNLock, LW_EXCLUSIVE);
-
    pairingheap_add(&waitLSN->waitersHeap, &procInfo->phNode);
    procInfo->inHeap = true;
    updateMinWaitedLSN();
@@ -203,6 +203,12 @@ WaitLSNSetLatches(XLogRecPtr currentLSN)
 void
 WaitLSNCleanup(void)
 {
+   /*
+    * We do a fast-path check of the 'inHeap' flag without the lock.  This
+    * flag is set to true only by the process itself.  So, it's only possible
+    * to get a false positive.  But that will be eliminated by a recheck
+    * inside deleteLSNWaiter().
+    */
    if (waitLSN->procInfos[MyProcNumber].inHeap)
        deleteLSNWaiter();
 }

diff --git a/src/include/commands/waitlsn.h b/src/include/commands/waitlsn.h
index b3d9eed64d816cfd0abc68708ba3317c3c2a9d6e..da17b8be6f9b5223d064dba3f461f01d23813a72 100644 (file)
--- a/src/include/commands/waitlsn.h
+++ b/src/include/commands/waitlsn.h
@@ -49,7 +49,7 @@ typedef struct WaitLSNState
    /*
     * The minimum LSN value some process is waiting for.  Used for the
     * fast-path checking if we need to wake up any waiters after replaying a
-    * WAL record.
+    * WAL record.  Could be read lock-less.  Update protected by WaitLSNLock.
     */
    pg_atomic_uint64 minWaitedLSN;
 
@@ -59,7 +59,10 @@ typedef struct WaitLSNState
     */
    pairingheap waitersHeap;
 
-   /* An array with per-process information, indexed by the process number */
+   /*
+    * An array with per-process information, indexed by the process number.
+    * Protected by WaitLSNLock.
+    */
    WaitLSNProcInfo procInfos[FLEXIBLE_ARRAY_MEMBER];
 } WaitLSNState;