Re: Reducing ClogControlLock contention

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?

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.

TransactionIdSetStatusBit()
{
..
/* note this assumes exclusive access to the clog page */
byteval = *byteptr;
byteval &= ~(((1 << CLOG_BITS_PER_XACT) - 1) << bshift);
byteval |= (status << bshift);
*byteptr = byteval;
..
}

Now even if this is a problem, I think we can solve it with some more lower
level lock or may be with atomic operation, but I have mentioned it to check
your opinion on the same.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com