Re: Reducing ClogControlLock contention - Mailing list pgsql-hackers
From | Simon Riggs |
---|---|
Subject | Re: Reducing ClogControlLock contention |
Date | |
Msg-id | CANP8+jJdLKiKFraU5Zk124r2MOagLSUQBLspgJ2cbF=A-3SwZg@mail.gmail.com Whole thread Raw |
In response to | Re: Reducing ClogControlLock contention (Amit Kapila <amit.kapila16@gmail.com>) |
Responses |
Re: Reducing ClogControlLock contention
(Amit Kapila <amit.kapila16@gmail.com>)
|
List | pgsql-hackers |
On 11 August 2015 at 10:55, Amit Kapila <amit.kapila16@gmail.com> wrote:
--
On Wed, Jul 1, 2015 at 3:49 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
>
> On 1 July 2015 at 11:14, Andres Freund <andres@anarazel.de> wrote:
>>
>> On 2015-07-01 09:08:11 +0100, Simon Riggs wrote:
>> > On 1 July 2015 at 09:00, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> >
>> > > On Tue, Jun 30, 2015 at 12:32 PM, Simon Riggs <simon@2ndquadrant.com>
>> > > a. the semantics of new LWLock (CommitLock) introduced
>> > > by patch seems to be different in the sense that it is just taken in
>> > > Exclusive mode (and no Shared mode is required) as per your proposal. We
>> > > could use existing LWLock APi's, but on the other hand we could even
>> > > invent new LWLock API for this kind of locking.
>> > >
>> >
>> > LWLock API code is already too complex, so -1 for more changes there
>>
>> I don't think that's a valid argument. It's better to have the
>> complexity in one place (lwlock) than have rather similar complexity in
>> several other places. The clog control lock is far from the only place
>> that would benefit from tricks along these lines.
>
>
> 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 lockingis that now in functions TransactionIdSetPageStatus andTransactionIdSetStatusBit, we modify slru's shared state with Control Lockin Shared mode whereas I think it is mandated in the code that thoseshould be modified with ControlLock in Exlusive mode. This could havesome 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 thantwo, so one effect of this patch will be that currently while doing I/O,concurrent committers will be allowed to proceed as we release ControlLockbefore doing the same whereas with Patch, they will not be allowed as theyare blocked by CommitLock. Now may be this scenario is less common anddoesn't matter much if the patch improves the more common scenario,however this is an indication of what Andres tries to highlight that having morelocks for this might make patch more complicated.
It's easy to stripe the CommitLock in that case, if it is a problem.
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pgsql-hackers by date: