Re: Reducing ClogControlLock contention

On 11 August 2015 at 10:55, Amit Kapila <amit.kapila16@gmail.com> wrote:

> What "tricks" are being used??
>
> Please explain why taking 2 locks is bad here, yet works fine elsewhere.
>

One thing that could be risky in this new scheme of locking

is that now in functions TransactionIdSetPageStatus and

TransactionIdSetStatusBit, we modify slru's shared state with Control Lock

in Shared mode whereas I think it is mandated in the code that those

should be modified with ControlLock in Exlusive mode.  This could have

some repercussions.

Do you know of any? This is a technical forum, so if we see a problem we say what it is, and if we don't, that's usually classed as a positive point in a code review. 

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.