Clarify what is protected by WaitLSNLock

akorotkov · akorotkov · commit ee79928441e7 · 2024-04-07T00:49:53.000+03:00
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
@@ -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
@@ -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;
 
