releaseOk and LWLockWaitForVar

On Tue, Jun 17, 2014 at 5:47 PM, Andres Freund <andres@2ndquadrant.com>
wrote:

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?

I also think above mentioned scenario is a problem if releaseOk
is not set to true in above case.

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.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jun 23, 2014 at 10:10 PM, Heikki Linnakangas <
hlinnakangas@vmware.com> wrote:

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.

Even if we don't want to add new trace points for new usage, I
think using existing might give wrong information to people who
want to use Dynamic tracing.

Another thing I have noticed is that docs are not updated for trace
macro's
TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT
TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL

Information related to existing tracing macro's is present at below link:
http://www.postgresql.org/docs/devel/static/dynamic-trace.html

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com