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. 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. > 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. 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. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com