Re: Condition variable live lock

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Condition variable live lock
Date: 2018-01-05 05:47:54
Message-ID: CAEepm=10Es09VfW656CXMnN0LENrAD4Z1Pn8Uxb9rpmDp+0i3Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 5, 2018 at 5:27 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I share Robert's discomfort with that solution, but it seems to me there
> might be a better way. The attached patch uses our own cvWaitLink as a
> sentinel to detect when we've woken everybody who was on the wait list
> before we arrived. That gives exactly the desired semantics, not just an
> approximation to them.

Very clever. It works correctly for my test case.

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

I think that restriction is probably OK. Even if you have some kind
of chain of CVs where you get woken up, check your interesting
condition and discover that it's now true so you exit you loop and
immediately want to broadcast a signal to some other CV, you'd simply
have to make sure that you put ConditionVariableCancelSleep() before
ConditionVariableBroadcast():

ConditionVariablePrepareToSleep(cv1);
while (condition for which we are waiting is not true)
ConditionVariableSleep(cv1, wait_event_info);
ConditionVariableCancelSleep();
ConditionVariableBroadcast(cv2);

It would only be a problem if you are interested in broadcasting to
cv2 when you've been woken up and the condition is *still not true*,
that is, when you've been spuriously woken. But why would anyone want
to forward spurious wakeups to another CV?

But if that seems too arbitrary, 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.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Vaishnavi Prabakaran 2018-01-05 05:55:50 Re: [HACKERS] PATCH: Batch/pipelining support for libpq
Previous Message Haribabu Kommi 2018-01-05 05:32:05 Re: [HACKERS] Refactor handling of database attributes between pg_dump and pg_dumpall