pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Shawn Debnath <sdn@amazon.com>
To: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Introduce timeout capability for ConditionVariableSleep
Date: Tue, 12 Mar 2019 16:24:54 -0700
Message-ID: <eeb06007ccfe46e399df6af18bfcd15a@EX13D05UWC002.ant.amazon.com> (raw)

Hello,

Postgres today doesn't support waiting for a condition variable with a 
timeout, although the framework it relies upon, does. This change wraps 
the existing ConditionVariableSleep functionality and introduces a new 
API, ConditionVariableTimedSleep, to allow callers to specify a timeout 
value.

A scenario that highlights this use case is a backend is waiting on 
status update from multiple workers but needs to time out if that signal 
doesn't arrive within a certain period. There was a workaround prior 
to aced5a92, but with that change, the semantics are now different.

I chose to go with -1 instead of 0 for the return from 
ConditionVariableTimedSleep to indicate timeout error  as it seems 
cleaner for this API. WaitEventSetWaitBlock returns -1 for timeout but 
WaitEventSetWait treats timeout as 0 (to represent 0 events indicating 
timeout).

If there's an alternative, cleaner way to achieve this outcome, I am all 
ears.

Thanks.

-- 
Shawn Debnath
Amazon Web Services (AWS)
From c1b332826070de4782b12bbb577d66406d439c8d Mon Sep 17 00:00:00 2001
From: Shawn Debnath <sdn@amazon.com>
Date: Tue, 12 Mar 2019 23:21:41 +0000
Subject: [PATCH] Introduce timeout capability for ConditionVariableSleep

This patch introduces ConditionVariableTimedSleep to enable timing out
of waiting for a condition variable to get signalled.
---
 src/backend/storage/lmgr/condition_variable.c | 41 +++++++++++++++++++++++++--
 src/include/storage/condition_variable.h      |  2 ++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index 58b7b51472..a0e14a5efa 100644
--- a/src/backend/storage/lmgr/condition_variable.c
+++ b/src/backend/storage/lmgr/condition_variable.c
@@ -121,9 +121,33 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv)
  */
 void
 ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
+{
+	(void) ConditionVariableTimedSleep(cv, -1 /* no timeout */, wait_event_info);
+}
+
+/*
+ * Wait for the given condition variable to be signaled or till timeout.
+ * This should be called in a predicate loop that tests for a specific exit
+ * condition and otherwise sleeps, like so:
+ *
+ *	 ConditionVariablePrepareToSleep(cv);  // optional
+ *	 while (condition for which we are waiting is not true)
+ *		 ConditionVariableSleep(cv, wait_event_info);
+ *	 ConditionVariableCancelSleep();
+ *
+ * wait_event_info should be a value from one of the WaitEventXXX enums
+ * defined in pgstat.h.  This controls the contents of pg_stat_activity's
+ * wait_event_type and wait_event columns while waiting.
+ *
+ * Returns 0 or -1 if timed out.
+ */
+int
+ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
+							uint32 wait_event_info)
 {
 	WaitEvent	event;
 	bool		done = false;
+	int			ret;
 
 	/*
 	 * If the caller didn't prepare to sleep explicitly, then do so now and
@@ -143,7 +167,7 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
 	if (cv_sleep_target != cv)
 	{
 		ConditionVariablePrepareToSleep(cv);
-		return;
+		return 0;
 	}
 
 	do
@@ -154,12 +178,23 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
 		 * Wait for latch to be set.  (If we're awakened for some other
 		 * reason, the code below will cope anyway.)
 		 */
-		(void) WaitEventSetWait(cv_wait_event_set, -1, &event, 1,
+		ret = WaitEventSetWait(cv_wait_event_set, timeout, &event, 1,
 								wait_event_info);
 
 		/* Reset latch before examining the state of the wait list. */
 		ResetLatch(MyLatch);
 
+		/* Timeout */
+		if (ret == 0 && timeout >= 0)
+		{
+			/*
+			 * In the event of a timeout, we simply return and the caller
+			 * calls ConditionVariableCancelSleep to remove themselves from the
+			 * wait queue
+			 */
+			return -1;
+		}
+
 		/*
 		 * If this process has been taken out of the wait list, then we know
 		 * that it has been signaled by ConditionVariableSignal (or
@@ -183,6 +218,8 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
 		}
 		SpinLockRelease(&cv->mutex);
 	} while (!done);
+
+	return 0;
 }
 
 /*
diff --git a/src/include/storage/condition_variable.h b/src/include/storage/condition_variable.h
index 2a0249392c..f62164e483 100644
--- a/src/include/storage/condition_variable.h
+++ b/src/include/storage/condition_variable.h
@@ -43,6 +43,8 @@ extern void ConditionVariableInit(ConditionVariable *cv);
  * the condition variable.
  */
 extern void ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info);
+extern int ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
+										uint32 wait_event_info);
 extern void ConditionVariableCancelSleep(void);
 
 /*
-- 
2.16.5

Attachments:

0001-Introduce-timeout-capability-for-ConditionVariableSleep-v1.patch (text/plain, 3.7K) download From c1b332826070de4782b12bbb577d66406d439c8d Mon Sep 17 00:00:00 2001 From: Shawn Debnath <sdn@amazon.com> Date: Tue, 12 Mar 2019 23:21:41 +0000 Subject: [PATCH] Introduce timeout capability for ConditionVariableSleep This patch introduces ConditionVariableTimedSleep to enable timing out of waiting for a condition variable to get signalled. --- src/backend/storage/lmgr/condition_variable.c | 41 +++++++++++++++++++++++++-- src/include/storage/condition_variable.h | 2 ++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c index 58b7b51472..a0e14a5efa 100644 --- a/src/backend/storage/lmgr/condition_variable.c +++ b/src/backend/storage/lmgr/condition_variable.c @@ -121,9 +121,33 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv) */ void ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info) +{ + (void) ConditionVariableTimedSleep(cv, -1 /* no timeout */, wait_event_info); +} + +/* + * Wait for the given condition variable to be signaled or till timeout. + * This should be called in a predicate loop that tests for a specific exit + * condition and otherwise sleeps, like so: + * + * ConditionVariablePrepareToSleep(cv); // optional + * while (condition for which we are waiting is not true) + * ConditionVariableSleep(cv, wait_event_info); + * ConditionVariableCancelSleep(); + * + * wait_event_info should be a value from one of the WaitEventXXX enums + * defined in pgstat.h. This controls the contents of pg_stat_activity's + * wait_event_type and wait_event columns while waiting. + * + * Returns 0 or -1 if timed out. + */ +int +ConditionVariableTimedSleep(ConditionVariable *cv, long timeout, + uint32 wait_event_info) { WaitEvent event; bool done = false; + int ret; /* * If the caller didn't prepare to sleep explicitly, then do so now and @@ -143,7 +167,7 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info) if (cv_sleep_target != cv) { ConditionVariablePrepareToSleep(cv); - return; + return 0; } do @@ -154,12 +178,23 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info) * Wait for latch to be set. (If we're awakened for some other * reason, the code below will cope anyway.) */ - (void) WaitEventSetWait(cv_wait_event_set, -1, &event, 1, + ret = WaitEventSetWait(cv_wait_event_set, timeout, &event, 1, wait_event_info); /* Reset latch before examining the state of the wait list. */ ResetLatch(MyLatch); + /* Timeout */ + if (ret == 0 && timeout >= 0) + { + /* + * In the event of a timeout, we simply return and the caller + * calls ConditionVariableCancelSleep to remove themselves from the + * wait queue + */ + return -1; + } + /* * If this process has been taken out of the wait list, then we know * that it has been signaled by ConditionVariableSignal (or @@ -183,6 +218,8 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info) } SpinLockRelease(&cv->mutex); } while (!done); + + return 0; } /* diff --git a/src/include/storage/condition_variable.h b/src/include/storage/condition_variable.h index 2a0249392c..f62164e483 100644 --- a/src/include/storage/condition_variable.h +++ b/src/include/storage/condition_variable.h @@ -43,6 +43,8 @@ extern void ConditionVariableInit(ConditionVariable *cv); * the condition variable. */ extern void ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info); +extern int ConditionVariableTimedSleep(ConditionVariable *cv, long timeout, + uint32 wait_event_info); extern void ConditionVariableCancelSleep(void); /* -- 2.16.5

view thread (20+ messages)  latest in thread

Message-ID: <eeb06007ccfe46e399df6af18bfcd15a@EX13D05UWC002.ant.amazon.com>
Permalink:  ../eeb06007ccfe46e399df6af18bfcd15a@EX13D05UWC002.ant.amazon.com/
Also on:    postgresql.org/message-id/eeb06007ccfe46e399df6af18bfcd15a@EX13D05UWC002.ant.amazon.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: sdn@amazon.com
  Subject: Re: Introduce timeout capability for ConditionVariableSleep
  In-Reply-To: <eeb06007ccfe46e399df6af18bfcd15a@EX13D05UWC002.ant.amazon.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox