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 CAA4eK1KK84enA6JRp5phur2uGmYYWuNs16Rz1UY==Ff42TfUPQ@mail.gmail.com
Whole thread Raw
In response to Re: Wait free LW_SHARED acquisition - v0.2  (Andres Freund <andres@2ndquadrant.com>)
Responses Re: Wait free LW_SHARED acquisition - v0.2  (Andres Freund <andres@2ndquadrant.com>)
List pgsql-hackers
On Tue, Jun 17, 2014 at 3:56 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>
> On 2014-06-17 12:41:26 +0530, Amit Kapila wrote:
> > 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()
>
> Hrmpf. Will fix.
>
> > 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.
>
> I don't understand this step here? When releasing the lock it'll notice
> that the waiters is <> 0 and acquire the spinlock which should protect
> against badness here?

While Releasing lock, I think it will not go to Wakeup waiters
(LWLockWakeup), because releaseOK will be false.  releaseOK
can be set as false when Session-1 has Released Exclusive lock
and wakedup some previous waiter.  Once it is set to false, it can
be reset to true only for retry logic(after getting semaphore).

> > 3.
> I don't think there's dangers here, lwWaiting will only ever be
> manipulated by the the PGPROC's owner. As discussed elsewhere there
> needs to be a write barrier before the proc->lwWaiting = false, even in
> upstream code.

Agreed. 


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

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: releaseOk and LWLockWaitForVar
Next
From: Hannu Krosing
Date:
Subject: Re: UPDATE SET (a,b,c) = (SELECT ...) versus rules