On Tue, Aug 11, 2015 at 4:09 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Aug 11, 2015 at 3:44 PM, Simon Riggs <si...@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? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
