postgres.git / summary / log / commit / refs

commit    acc5c4fd8f83e5991cab11d7299d112e89cb3fe7
Author:   Thomas Munro <tmunro@postgresql.org>
Date:     Mon Aug 14 22:20:11 2023 +0000
First released in: 17.0, 16.0

    De-pessimize ConditionVariableCancelSleep().
    
    Commit b91dd9de was concerned with a theoretical problem with our
    non-atomic condition variable operations.  If you stop sleeping, and
    then cancel the sleep in a separate step, you might be signaled in
    between, and that could be lost.  That doesn't matter for callers of
    ConditionVariableBroadcast(), but callers of ConditionVariableSignal()
    might be upset if a signal went missing like this.
    
    Commit bc971f4025c interacted badly with that logic, because it doesn't
    use ConditionVariableSleep(), which would normally put us back in the
    wait list.  ConditionVariableCancelSleep() would be confused and think
    we'd received an extra signal, and try to forward it to another backend,
    resulting in wakeup storms.
    
    New idea: ConditionVariableCancelSleep() can just return true if we've
    been signaled.  Hypothetical users of ConditionVariableSignal() would
    then still have a way to deal with rare lost signals if they are
    concerned about that problem.
    
    Back-patch to 16, where bc971f4025c arrived.
    
    Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
    Reviewed-by: Andres Freund <andres@anarazel.de>
    Discussion: https://postgr.es/m/2840876b-4cfe-240f-0a7e-29ffd66711e7%40enterprisedb.com


 src/backend/storage/lmgr/condition_variable.c | 16 ++++++----------
 src/include/storage/condition_variable.h      |  2 +-
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index 7e2bbf46d9d..910a768206f 100644
--- a/src/backend/storage/lmgr/condition_variable.c
+++ b/src/backend/storage/lmgr/condition_variable.c
@@ -223,15 +223,17 @@ ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
  *
  * Do nothing if nothing is pending; this allows this function to be called
  * during transaction abort to clean up any unfinished CV sleep.
+ *
+ * Return true if we've been signaled.
  */
-void
+bool
 ConditionVariableCancelSleep(void)
 {
 	ConditionVariable *cv = cv_sleep_target;
 	bool		signaled = false;
 
 	if (cv == NULL)
-		return;
+		return false;
 
 	SpinLockAcquire(&cv->mutex);
 	if (proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink))
@@ -240,15 +242,9 @@ ConditionVariableCancelSleep(void)
 		signaled = true;
 	SpinLockRelease(&cv->mutex);
 
-	/*
-	 * If we've received a signal, pass it on to another waiting process, if
-	 * there is one.  Otherwise a call to ConditionVariableSignal() might get
-	 * lost, despite there being another process ready to handle it.
-	 */
-	if (signaled)
-		ConditionVariableSignal(cv);
-
 	cv_sleep_target = NULL;
+
+	return signaled;
 }
 
 /*
diff --git a/src/include/storage/condition_variable.h b/src/include/storage/condition_variable.h
index 589bdd323cb..e218cb2c499 100644
--- a/src/include/storage/condition_variable.h
+++ b/src/include/storage/condition_variable.h
@@ -56,7 +56,7 @@ extern void ConditionVariableInit(ConditionVariable *cv);
 extern void ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info);
 extern bool ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
 										uint32 wait_event_info);
-extern void ConditionVariableCancelSleep(void);
+extern bool ConditionVariableCancelSleep(void);
 
 /*
  * Optionally, ConditionVariablePrepareToSleep can be called before entering


[parent: 03fb43f6ed63]