Re: Introduce timeout capability for ConditionVariableSleep - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: Introduce timeout capability for ConditionVariableSleep
Date
Msg-id CA+hUKGJDt3yopu1K1xPW5NtbLq-0n6fbXxZcaOpqD0P0-CruyA@mail.gmail.com
Whole thread Raw
In response to Introduce timeout capability for ConditionVariableSleep  (Shawn Debnath <sdn@amazon.com>)
Responses Re: Introduce timeout capability for ConditionVariableSleep
List pgsql-hackers
Hi Shawn,

On Wed, Mar 13, 2019 at 12:25 PM Shawn Debnath <sdn@amazon.com> wrote:
> 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.

Seems reasonable, I think, and should be familiar to anyone used to
well known multithreading libraries.

+/*
+ * 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)


Can we just refer to the other function's documentation for this?  I
don't want two copies of this blurb (and this copy-paste already
failed to include "Timed" in the example function name).

One difference compared to pthread_cond_timedwait() is that pthread
uses an absolute time and here you use a relative time (as we do in
WaitEventSet API).  The first question is which makes a better API,
and the second is what the semantics of a relative timeout should be:
start again every time we get a spurious wake-up, or track time
already waited?  Let's see...

-        (void) WaitEventSetWait(cv_wait_event_set, -1, &event, 1,
+        ret = WaitEventSetWait(cv_wait_event_set, timeout, &event, 1,
                                 wait_event_info);

Here you're restarting the timeout clock every time through the loop
without adjustment, and I think that's not a good choice: spurious
wake-ups cause bonus waiting.

-- 
Thomas Munro
https://enterprisedb.com


pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Should we add GUCs to allow partition pruning to be disabled?
Next
From: Amit Langote
Date:
Subject: Re: Update does not move row across foreign partitions in v11