Date: Thu, 21 Mar 2019 14:20:50 -0400
From: Shawn Debnath <sdn@amazon.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
CC: <thomas.munro@gmail.com>, <pgsql-hackers@postgresql.org>
Subject: Re: Introduce timeout capability for ConditionVariableSleep
Message-ID: <20190321182049.GA95415@f01898859afd.ant.amazon.com>
References: <20190313005342.GA8301@f01898859afd.ant.amazon.com>
 <20190313.172415.156658364.horiguchi.kyotaro@lab.ntt.co.jp>
 <20190315002611.GA1119@f01898859afd.ant.amazon.com>
 <20190315.141517.229371570.horiguchi.kyotaro@lab.ntt.co.jp>
 <20190316222717.GA2139@f01898859afd.ant.amazon.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="HlL+5n6rz5pIUxbD"
Content-Disposition: inline
In-Reply-To: <20190316222717.GA2139@f01898859afd.ant.amazon.com>
User-Agent: Mutt/1.11.3 (2019-02-01)
Precedence: bulk

--HlL+5n6rz5pIUxbD
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline

On Sat, Mar 16, 2019 at 03:27:17PM -0700, Shawn Debnath wrote:
> > +     * Track the current time so that we can calculate the 
> > remaining timeout
> > +     * if we are woken up spuriously.
> > 
> > I think tha "track" means chasing a moving objects. So it might
> > be bettter that it is record or something?
> > 
> > >   * Wait for the given condition variable to be signaled or till timeout.
> > >   *
> > >   * Returns -1 when timeout expires, otherwise returns 0.
> > >   *
> > >   * See ConditionVariableSleep() for general usage.
> > > 
> > > > +ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
> > > > 
> > > > Counldn't the two-state return value be a boolean?
> 
> I will change it to Record in the next iteration of the patch.

Posting rebased and updated patch. Changed the word 'Track' to 'Record' 
and also changed variable name rem_timeout to cur_timeout to match 
naming in other use cases.


-- 
Shawn Debnath
Amazon Web Services (AWS)

--HlL+5n6rz5pIUxbD
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: attachment;
	filename="0001-Introduce-timeout-capability-for-ConditionVariableSleep-v4.patch"

From 537e879a66f7d59333ede62e8f7c42fec4006a31 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 | 63 ++++++++++++++++++++++++---
 src/include/storage/condition_variable.h      |  2 +
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index 58b7b51472..390072451c 100644
--- a/src/backend/storage/lmgr/condition_variable.c
+++ b/src/backend/storage/lmgr/condition_variable.c
@@ -19,6 +19,7 @@
 #include "postgres.h"
 
 #include "miscadmin.h"
+#include "portability/instr_time.h"
 #include "storage/condition_variable.h"
 #include "storage/ipc.h"
 #include "storage/proc.h"
@@ -122,8 +123,23 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv)
 void
 ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
 {
-	WaitEvent	event;
-	bool		done = false;
+	(void) ConditionVariableTimedSleep(cv, -1 /* no timeout */, wait_event_info);
+}
+
+/*
+ * Wait for the given condition variable to be signaled or till timeout.
+ *
+ * Returns true when timeout expires, otherwise returns false.
+ *
+ * See ConditionVariableSleep() for general usage.
+ */
+bool
+ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
+							uint32 wait_event_info)
+{
+	long		cur_timeout = -1;
+	instr_time	start_time;
+	instr_time	cur_time;
 
 	/*
 	 * If the caller didn't prepare to sleep explicitly, then do so now and
@@ -143,23 +159,42 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
 	if (cv_sleep_target != cv)
 	{
 		ConditionVariablePrepareToSleep(cv);
-		return;
+		return false;
 	}
 
-	do
+	/*
+	 * Record the current time so that we can calculate the remaining timeout
+	 * if we are woken up spuriously.
+	 */
+	if (timeout >= 0)
 	{
+		INSTR_TIME_SET_CURRENT(start_time);
+		Assert(timeout >= 0 && timeout <= INT_MAX);
+		cur_timeout = timeout;
+	}
+
+	while (true)
+	{
+		WaitEvent	event;
+		bool		done = false;
+		int 		rc;
+
 		CHECK_FOR_INTERRUPTS();
 
 		/*
 		 * 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,
+		rc = WaitEventSetWait(cv_wait_event_set, cur_timeout, &event, 1,
 								wait_event_info);
 
 		/* Reset latch before examining the state of the wait list. */
 		ResetLatch(MyLatch);
 
+		/* Timed out */
+		if (rc == 0)
+			return true;
+
 		/*
 		 * If this process has been taken out of the wait list, then we know
 		 * that it has been signaled by ConditionVariableSignal (or
@@ -182,7 +217,23 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
 			proclist_push_tail(&cv->wakeup, MyProc->pgprocno, cvWaitLink);
 		}
 		SpinLockRelease(&cv->mutex);
-	} while (!done);
+
+		/* We were signaled, so return */
+		if (done)
+			return false;
+
+		/* If we're not done, update cur_timeout for next iteration */
+		if (timeout >= 0)
+		{
+			INSTR_TIME_SET_CURRENT(cur_time);
+			INSTR_TIME_SUBTRACT(cur_time, start_time);
+			cur_timeout = timeout - (long) INSTR_TIME_GET_MILLISEC(cur_time);
+
+			/* Have we crossed the timeout threshold? */
+			if (cur_timeout <= 0)
+				return true;
+		}
+	}
 }
 
 /*
diff --git a/src/include/storage/condition_variable.h b/src/include/storage/condition_variable.h
index 2a0249392c..ee06e051ce 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 bool ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
+										uint32 wait_event_info);
 extern void ConditionVariableCancelSleep(void);
 
 /*
-- 
2.16.5


--HlL+5n6rz5pIUxbD--