On 1 July 2015 at 09:00, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Jun 30, 2015 at 12:32 PM, Simon Riggs <si...@2ndquadrant.com> > wrote: > > > > ClogControlLock contention is high at commit time. This appears to be > due to the fact that ClogControlLock is acquired in Exclusive mode prior to > marking commit, which then gets starved by backends running > TransactionIdGetStatus(). > > > > Proposal for improving this is to acquire the ClogControlLock in Shared > mode, if possible. > > > > This approach looks good way for avoiding the contention around > ClogControlLock. Few things that occurred to me while looking at > patch are that > > 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 > b. SimpleLruReadPage_ReadOnly() - This API's is meant mainly for > read-access of page and the description also says the same, but now > we want to use it for updating page as well. It might be better to invent > similar new API or at the very least modify it's description. > Agreed, perhaps SimpleLruReadPage_Optimistic() > > Two concurrent writers might access the same word concurrently, so we > protect against that with a new CommitLock. We could partition that by > pageno also, if needed. > > > > I think it will be better to partition it or use it in some other way to > avoid > two concurrent writers block at it, however if you want to first see the > test results with this, then that is also okay. > Many updates would be on same page, so partitioning it would need to be at least 4-way to be worth doing. Maybe we could stripe into 512 bye pages. > Overall the idea seems good to pursue, however I have slight feeling > that using 2 LWLocks (CLOGControlLock in shared mode and new > CommitLock in Exclusive mode) to set the transaction information > is somewhat odd, but I could not see any problem with it. > Perhaps call it the CommitSerializationLock would help. There are already locks that are held only in Exclusive mode. Locking two separate resources at same time is common in other code. -- Simon Riggs http://www.2ndQuadrant.com/ <http://www.2ndquadrant.com/> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
