On Tue, Jun 17, 2014 at 6:35 PM, Andres Freund <and...@2ndquadrant.com>
wrote:
>
> On 2014-06-17 18:01:58 +0530, Amit Kapila wrote:
> > On Tue, Jun 17, 2014 at 3:56 PM, Andres Freund <and...@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.
You have followed it pretty well as far as I can understand from your
replies, as there is no reproducible test (which I think is bit tricky to
prepare), so it becomes difficult to explain by theory.
> 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?
You are right, it will wakeup the existing waiters, but I think the
new logic has one difference which is that it can allow the backend to
take Exclusive lock when there are already waiters in queue. As per
above example even though Session-2 and Session-3 are in wait
queue, Session-4 will be able to acquire Exclusive lock which I think
was previously not possible.
> 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.
I think it will also maintain that the wokedup process won't stall for
very long time, because if we wake new waiters, then previously woked
process can again enter into wait queue and similar thing can repeat
for long time.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
