On Tue, Aug 11, 2015 at 4:09 PM, Amit Kapila <
amit.kapila16@gmail.com> wrote:
>
> On Tue, Aug 11, 2015 at 3:44 PM, Simon Riggs <
simon@2ndquadrant.com> wrote:
>>
>>>
>>> Another thing is that in this flow, with patch there will be three locks
>>> (we take per-buffer locks before doing I/O) that will get involved rather than
>>> two, so one effect of this patch will be that currently while doing I/O,
>>> concurrent committers will be allowed to proceed as we release ControlLock
>>> before doing the same whereas with Patch, they will not be allowed as they
>>> are blocked by CommitLock. Now may be this scenario is less common and
>>> doesn't matter much if the patch improves the more common scenario,
>>> however this is an indication of what Andres tries to highlight that having more
>>> locks for this might make patch more complicated.
>>
>>
>> It's easy to stripe the CommitLock in that case, if it is a problem.
>
>
> Sure, I think other places in code that take both the other locks also
> needs to be checked for updation.
>
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?