Re: Introduce timeout capability for ConditionVariableSleep

Hi Shawn,

On Wed, Mar 13, 2019 at 12:25 PM Shawn Debnath <sdn(at)amazon(dot)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