On 8 March 2017 at 10:02, David Rowley <david.rowley@2ndquadrant.com> wrote: > On 8 March 2017 at 01:13, Simon Riggs <simon@2ndquadrant.com> wrote: >> Don't understand this. I'm talking about setting a flag on >> commit/abort WAL records, like the attached. > > There's nothing setting a flag in the attached. I only see the > checking of the flag.
Yeh, it wasn't a full patch, just a way marker to the summit for you. >> We just need to track info so we can set the flag at EOXact and we're done. > > Do you have an idea where to store that information? I'd assume we'd > want to store something during LogAccessExclusiveLocks() and check > that in XactLogCommitRecord() and XactLogAbortRecord(). I don't see > anything existing which might help with that today. Do you think I > should introduce some new global variable for that? Or do you propose > calling GetRunningTransactionLocks() again while generating the > Commit/Abort record?
Seemed easier to write it than explain further. Please see what you think.
Thanks. I had been looking for some struct to store the flag in. I'd not considered just adding a new global variable.
I was in fact just working on this again earlier, and I had come up with the attached.
I was just trying to verify if it was going to do the correct thing in regards to subtransactions and I got a little confused at the comments at the top of StandbyAcquireAccessExclusiveLock():
* We record the lock against the top-level xid, rather than individual
* subtransaction xids. This means AccessExclusiveLocks held by aborted
* subtransactions are not released as early as possible on standbys.
if this is true, and it seems to be per LogAccessExclusiveLock(), does StandbyReleaseLockTree() need to release the locks for sub transactions too?
This code:
for (i = 0; i < nsubxids; i++)
StandbyReleaseLocks(subxids[i]);
FWIW I tested the attached and saw that with my test cases I showed earlier that StandbyReleaseLockTree() was down to 0.74% in the perf report. Likely the gain will be even better if I ran a few more CPUs on small simple transactions which are not taking Access Exclusive locks, so I agree this is a better way forward than what I had in my original patch. Your patch will have the same effect, so much better than the 1.74% I saw in my perf report for the 1024 bucket hash table.
