From: | Shawn Debnath <sdn(at)amazon(dot)com> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Introduce timeout capability for ConditionVariableSleep |
Date: | 2019-03-13 00:53:43 |
Message-ID: | 20190313005342.GA8301@f01898859afd.ant.amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Thomas,
Thanks for reviewing!
On Wed, Mar 13, 2019 at 12:40:57PM +1300, Thomas Munro wrote:
> 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).
Hah - point well taken. Fixed.
> 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.
Agree. In my testing WaitEventSetWait did the work for calculating the
right timeout remaining. It's a bummer that we have to repeat the same
pattern at the ConditionVariableTimedSleep() but I guess anyone who
loops in such cases will have to adjust their values accordingly.
BTW, I am curious why Andres in 98a64d0bd71 didn't just create an
artificial event with WL_TIMEOUT and return that from
WaitEventSetWait(). Would have made it cleaner than checking checking
return values for -1.
Updated v2 patch attached.
--
Shawn Debnath
Amazon Web Services (AWS)
Attachment | Content-Type | Size |
---|---|---|
0001-Introduce-timeout-capability-for-ConditionVariableSleep-v2.patch | text/plain | 4.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Shawn Debnath | 2019-03-13 01:00:07 | Re: Refactoring the checkpointer's fsync request queue |
Previous Message | Tatsuo Ishii | 2019-03-13 00:45:27 | Re: Early WIP/PoC for inlining CTEs |