Re: condition variables - Mailing list pgsql-hackers

On Mon, Nov 21, 2016 at 7:10 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
>> I wonder if we shouldn't try to create the invariant that when the CV
>> mutex is not help, the state of cvSleeping has to be true if we're in
>> the proclist and false if we're not.  So ConditionVariableSignal()
>> would clear the flag before releasing the spinlock, for example.  Then
>> I think we wouldn't need proclist_contains().
>
> Yeah, that'd work.  ConditionVariableSleep would need to hold the
> spinlock while checking cvSleeping (otherwise there is race case where
> another process sets it to false but this process doesn't see that yet
> and then waits for a latch-set which never arrives).  It's not the end
> of the world but it seems unfortunate to have to acquire and release
> the spinlock in ConditionVariablePrepareToSleep and then immediately
> again in ConditionVariableSleep.
>
> I was going to code that up but I read this and had another idea:
>
> http://stackoverflow.com/questions/8594591/why-does-pthread-cond-wait-have-spurious-wakeups
>
> I realise that you didn't actually say you wanted to guarantee no
> spurious wakeups.  But since the client code already has to check its
> own exit condition, why bother adding a another layer of looping?
> Sprurious latch sets must be super rare, but even if they aren't, what
> do you save by filtering them here?  In this situation you already got
> woken from a deep slumbler and scheduled back onto the CPU, so it
> hardly matters whether you go around again in that loop or the
> client's loop.  We could make things really simple instead: get rid of
> cvSleeping, have ConditionVariablePrepareToSleep reset the latch, then
> have ConditionVariableSleep wait for the latch to be set just once (no
> loop).  Then we'd document that spurious wakeups are possible so the
> caller must write a robust predicate loop, exactly as you already
> showed in your first message.  We'd need to keep that
> proclist_contains stuff to avoid corrupting the list.
> proclist_contains would be the one source of truth for whether you're
> in the waitlist, and the client code's predicate loop would contain
> the one source of truth for whether the condition it is waiting for
> has been reached.

I don't think we can rely on spurious latch set events being rare.
There are an increasing number of things that set the process latch,
and there will very likely be more in the future.  For instance, the
arrival of tuples from a parallel worker associated with our session
will set the process latch.  Workers starting or dying will set the
process latch.  So my inclination was to try to guarantee no spurious
wakeups at all, but possibly a softer guarantee that makes them
unlikely would be sufficient.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company