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;