Re: Wait free LW_SHARED acquisition - v0.2

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Wait free LW_SHARED acquisition - v0.2
Date: 2014-06-17 07:11:26
Message-ID: CAA4eK1LZ6EfbevYfY6=1gLLz=TXMHAO_GQGYV3hG8NtBr34BhQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 23, 2014 at 10:01 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> On Fri, Jan 31, 2014 at 3:24 PM, Andres Freund <andres(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Martijn van Oosterhout 2014-06-17 07:18:18 Re: UPDATE SET (a,b,c) = (SELECT ...) versus rules
Previous Message Pavel Stehule 2014-06-17 06:51:21 Re: How to implement the skip errors for copy from ?