On Tue, Aug 11, 2015 at 7:27 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > > On 11 August 2015 at 14:53, Amit Kapila <amit.kapila16@gmail.com> wrote: > >> >> 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? > > > That prevents read only access, not just commits, so that isn't a better suggestion.
read only access of what (clog page?)?
Here we are mainly doing three operations read clog page, write transaction status
on clog page and update shared control state. So basically two resources are
involved clog page and shared control state, so which one of those you are talking?
Sorry, your suggestion was good. Using LWLockAcquire(shared->buffer_locks[slotno], LW_EXCLUSIVE); now seems sufficient.
Apart from above, in below code, it is assumed that we have exclusive lock on
clog page which we don't in the proposed patch as some one can read the
same page while we are modifying it. In current code, this assumption is valid
because during Write we take CLogControlLock in Exclusive mode and while
Reading we take the same in Shared mode.
Not exactly, no. This is not a general case, it is for one important and very specific case only, exactly suited to our transaction manager. I have checked all call paths and we are good.
New patch attached. I will reply to Andres' post separately since this does not yet address all of his detailed points.
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services