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

On 2014-06-17 18:01:58 +0530, Amit Kapila wrote:
> On Tue, Jun 17, 2014 at 3:56 PM, Andres Freund <andres@2ndquadrant.com>
> > On 2014-06-17 12:41:26 +0530, Amit Kapila wrote:
> > > 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).

I unfortunately still can't follow. If Session-1 woke up some previous
waiter the woken up process will set releaseOK to true again when it
loops to acquire the lock?


Somewhat unrelated:

I have a fair amount of doubt about the effectiveness of the releaseOK
logic (which imo also is pretty poorly documented).
Essentially its intent is to avoid unneccessary scheduling when other
processes have already been woken up (i.e. releaseOK has been set to
false). I believe the theory is that if any process has already been
woken up it's pointless to wake up additional processes
(i.e. PGSemaphoreUnlock()) because the originally woken up process will
wake up at some point. But if the to-be-woken up process is scheduled
out because it used all his last timeslices fully that means we'll not
wakeup other waiters for a relatively long time.

It's been introduced in the course of
5b9a058384e714b89e050fc0b6381f97037c665a whose logic generally is rather
sound - I just doubt that the releaseOK part is necessary.

It'd certainly interesting to rip releaseOK out and benchmark the
result... My theory is that the average latency will go down on busy
systems that aren't IO bound.

Greetings,

Andres Freund

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