Thread: releaseOk and LWLockWaitForVar

Hi Heikki, All,

Amit just pointed me to a case where the lwlock scalability patch
apparently causes problems and I went on to review it and came across
the following problem in 9.4/master:
LWLockWaitForVar() doesn't set releaseOk to true when waiting
again. Isn't that a bug? What if there's another locker coming in after
LWLockWaitForVar() returns from the PGSemaphoreLock() but before it has
acquire the spinlock? Now, it might be that it's unproblematic because
of hte specific way these locks are used right now, but it doesn't seem
like a good idea to leave it that way.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On 06/17/2014 03:17 PM, Andres Freund wrote:
> LWLockWaitForVar() doesn't set releaseOk to true when waiting
> again. Isn't that a bug?

LWLockWaitForVar() waits in LW_WAIT_UNTIL_FREE mode, because it's not 
interested in acquiring the lock, it just wants to be woken up when it's 
released (or the "var" is updated). LWLockRelease doesn't clear 
releaseOK when it wakes up a LW_WAIT_UNTIL_FREE-mode waiter.

> What if there's another locker coming in after
> LWLockWaitForVar() returns from the PGSemaphoreLock() but before it has
> acquire the spinlock? Now, it might be that it's unproblematic because
> of hte specific way these locks are used right now, but it doesn't seem
> like a good idea to leave it that way.

In that scenario, LWLockWaitForVar() will grab the spinlock, after the 
other process. What happens next depends on the whether the value of the 
variable it guards was changed. If it was, LWLockWaitForVar() will see 
that it changed, and return false without waiting again. If the value 
didn't change, it will sleep until the new locker releases the lock. In 
either case, I don't see a problem with releaseOK. It seems correct as 
it is.

- Heikki

On 06/23/2014 05:38 PM, Amit Kapila wrote:
> While looking at function LWLockWaitForVar(), espacially below
> code:
>
> TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(l), T_ID(l), LW_EXCLUSIVE);
>
> I think in this function tracing is done considering the Exclusive lock
> is acquired, however it might have granted access because of
> variable updation.  Basically this function's trace doesn't distinguish
> whether the access is granted due to the reason that there is no other
> exclusive locker or variable is updated.

Yeah. Not sure it's worth it to add new TRACE points for this, I'm not 
really familiar with the way the traces work or how people use them.

- Heikki