Re: Wait free LW_SHARED acquisition - v0.2 - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Wait free LW_SHARED acquisition - v0.2
Date
Msg-id CAA4eK1LZ6EfbevYfY6=1gLLz=TXMHAO_GQGYV3hG8NtBr34BhQ@mail.gmail.com
Whole thread Raw
In response to Re: Wait free LW_SHARED acquisition - v0.2  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Wait free LW_SHARED acquisition - v0.2  (Andres Freund <andres@2ndquadrant.com>)
List pgsql-hackers
On Fri, May 23, 2014 at 10:01 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, Jan 31, 2014 at 3:24 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> > I've pushed a rebased version of the patchset to
> > http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git
> > branch rwlock contention.
> > 220b34331f77effdb46798ddd7cca0cffc1b2858 actually was the small problem,
> > ea9df812d8502fff74e7bc37d61bdc7d66d77a7f was the major PITA.
>
> As per discussion in developer meeting, I wanted to test shared
> buffer scaling patch with this branch.  I am getting merge
> conflicts as per HEAD.  Could you please get it resolved, so that
> I can get the data.

I have started looking into this patch and have few questions/
findings which are shared below:

1. I think stats for lwstats->ex_acquire_count will be counted twice,
first it is incremented in LWLockAcquireCommon() and then in
LWLockAttemptLock()

2.
Handling of potentialy_spurious case seems to be pending
in LWLock functions like LWLockAcquireCommon().

LWLockAcquireCommon()
{
.. 
/* add to the queue */
LWLockQueueSelf(l, mode);

/* we're now guaranteed to be woken up if necessary */
mustwait = LWLockAttemptLock(l, mode, false, &potentially_spurious);

}

I think it can lead to some problems, example:

Session -1
1. Acquire Exclusive LWlock

Session -2
1. Acquire Shared LWlock

1a. Unconditionally incrementing shared count by session-2

Session -1
2. Release Exclusive lock

Session -3 
1. Acquire Exclusive LWlock
It will put itself to wait queue by seeing the lock count incremented
by Session-2

Session-2
1b. Decrement the shared count and add it to wait queue.

Session-4
1. Acquire Exclusive lock
   This session will get the exclusive lock, because even
   though other lockers are waiting, lockcount is zero.

Session-2
2. Try second time to take shared lock, it won't get
   as session-4 already has an exclusive lock, so it will
   start waiting

Session-4
2. Release Exclusive lock
   it will not wake the waiters because waiters have been added
   before acquiring this lock.

So in above scenario, Session-3 and Session-2 are waiting in queue
with nobody to awake them.

I have not reproduced the exact scenario above,
so I might be missing some thing which will not
lead to above situation.

3.
LWLockAcquireCommon()
{
for (;;)
{
PGSemaphoreLock(&proc->sem, false);
if (!proc->lwWaiting)
..
}
proc->lwWaiting is checked, updated without spinklock where
as previously it was done under spinlock, won't it be unsafe?

4.
LWLockAcquireCommon()
{
..
for (;;)
{
/* "false" means cannot accept cancel/die interrupt here. */
PGSemaphoreLock(&proc->sem, false);
if (!proc->lwWaiting)
break;
extraWaits++;
}
lock->releaseOK = true;
}

lock->releaseOK is updated/checked without spinklock where
as previously it was done under spinlock, won't it be unsafe?


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

pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: How to implement the skip errors for copy from ?
Next
From: Martijn van Oosterhout
Date:
Subject: Re: UPDATE SET (a,b,c) = (SELECT ...) versus rules