From ee79928441e7e291532b833455ebfee27d7cab5c Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Sun, 7 Apr 2024 00:32:35 +0300 Subject: [PATCH] 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 a57b818a2d..1a83c34e09 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 b3d9eed64d..da17b8be6f 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;