Horton

Re: Condition variable live lock

Thomas Munro <thomas.munro@enterprisedb.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2018-01-05T06:33:10Z
Lists: pgsql-hackers 
On Fri, Jan 5, 2018 at 7:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I thought of another possible issue, though.  In the situation where
> someone else has removed our sentinel (presumably, by issuing
> ConditionVariableSignal just before we were about to remove the
> sentinel), my patch assumes we can just do nothing.  But it seems
> like that amounts to losing one signal.  Whoever the someone else
> was probably expected to awaken a waiter, and now that won't happen.

Yeah, that's bad.

> Should we rejigger the logic so that it awakens one additional waiter
> (if there is one) after detecting that someone else has removed the
> sentinel?  Obviously, this trades a risk of loss of wakeup for a risk
> of spurious wakeup, but presumably the latter is something we can
> cope with.

One detail is that the caller of ConditionVariableSignal() got a true
return value when it took out the sentinel (indicating that someone
received the signal), and now when you call ConditionVariableSignal()
because !aminlist there may be no one there.  I'm not sure if that's a
problem.  For comparison, pthread_cond_signal() doesn't tell you if
you actually signalled anyone.  Maybe the only reason we have that
return code is so that ConditionVariableBroadcast() can use it the way
it does in master...

An alternative would be to mark sentinel entries somehow so that
signallers can detect them and signal again, but that's not
backpatchable.

-- 
Thomas Munro
http://www.enterprisedb.com

Commits

  1. Add WL_EXIT_ON_PM_DEATH pseudo-event.

  2. While waiting for a condition variable, detect postmaster death.

  3. Fix race condition during replication origin drop.

  4. Allow ConditionVariable[PrepareTo]Sleep to auto-switch between CVs.

  5. Cosmetic improvements in condition_variable.[hc].

  6. Improve error detection capability in proclists.

  7. Remove return values of ConditionVariableSignal/Broadcast.

  8. Reorder steps in ConditionVariablePrepareToSleep for more safety.

  9. Rewrite ConditionVariableBroadcast() to avoid live-lock.

  10. Add parallel-aware hash joins.