Re: Reducing ClogControlLock contention

On 12 August 2015 at 04:49, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> On Tue, Aug 11, 2015 at 7:27 PM, Simon Riggs <simon(at)2ndquadrant(dot)com>
> wrote:
> >
> > On 11 August 2015 at 14:53, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> >>
> >> One more point here why do we need CommitLock before calling
> >> SimpleLruReadPage_ReadOnly() in the patch and if it is not required,
> >> then can we use LWLockAcquire(shared->buffer_locks[slotno],
> LW_EXCLUSIVE);
> >> instead of CommitLock?
> >
> >
> > That prevents read only access, not just commits, so that isn't a better
> suggestion.
>
> read only access of what (clog page?)?
>
> Here we are mainly doing three operations read clog page, write
> transaction status
> on clog page and update shared control state. So basically two resources
> are
> involved clog page and shared control state, so which one of those you are
> talking?
>

Sorry, your suggestion was good. Using
LWLockAcquire(shared->buffer_locks[slotno], LW_EXCLUSIVE); now seems
sufficient.

Apart from above, in below code, it is assumed that we have exclusive lock
> on
> clog page which we don't in the proposed patch as some one can read the
> same page while we are modifying it. In current code, this assumption is
> valid
> because during Write we take CLogControlLock in Exclusive mode and while
> Reading we take the same in Shared mode.
>

Not exactly, no. This is not a general case, it is for one important and
very specific case only, exactly suited to our transaction manager. I have
checked all call paths and we are good.

New patch attached. I will reply to Andres' post separately since this does
not yet address all of his detailed points.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services