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
|
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.
> 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?
pgsql-hackers by date: