Horton

Re: Condition variable live lock

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2018-01-05T06:10:57Z
Lists: pgsql-hackers 
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Fri, Jan 5, 2018 at 5:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Now, the limitation with this is that we can't be waiting for any *other*
>> condition variable, because then we'd be trashing our state about that
>> variable.  As coded, we can't be waiting for the target CV either, but
>> that case could actually be handled if we needed to, as per the comment.
>> I do not know if this is likely to be a problematic limitation
>> ... discuss.  (The patch does survive check-world, FWIW.)

> ... one way to lift the restriction would
> be to teach ConditionVariableBroadcast() to call
> ConditionVariableCancelSleep() if cv_sleep_target is non-NULL where
> you have the current assertion.  Code that is still waiting for a CV
> must be in a loop that will eventually re-add it in
> ConditionVariableSleep(), and it won't miss any signals that it can't
> afford to miss because the first call to ConditionVariableSleep() will
> return immediately so the caller will recheck its condition.

Oh, of course, very simple.

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.
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.

			regards, tom lane

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.