postgres.git / summary / log / commit / refs

commit    ee79928441e7e291532b833455ebfee27d7cab5c
Author:   Alexander Korotkov <akorotkov@postgresql.org>
Date:     Sat Apr 06 21:32:35 2024 +0000
First released in: master (reverted before release)
Reverted by: 772faafca1b2 (in 17.0)

    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


 src/backend/commands/waitlsn.c | 10 ++++++++--
 src/include/commands/waitlsn.h |  7 +++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/waitlsn.c b/src/backend/commands/waitlsn.c
index a57b818a2d4..1a83c34e09f 100644
--- 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 b3d9eed64d8..da17b8be6f9 100644
--- 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;
 


[parent: 25f42429e2ff]