On Fri, Mar 6, 2020 at 9:47 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Thu, Mar 5, 2020 at 1:54 PM Dilip Kumar <dilipbalaut@gmail.com> wrote: > > > > On Thu, Mar 5, 2020 at 12:15 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > > > > > 5. I have also tried to think of another way to check if we already > > > hold lock type LOCKTAG_RELATION_EXTEND, but couldn't come up with a > > > cheaper way than this. Basically, I think if we traverse the > > > MyProc->myProcLocks queue, we will get this information, but that > > > doesn't seem much cheaper than this. > > > > I think we can maintain a flag (rel_extlock_held). And, we can set > > that true in LockRelationForExtension, > > ConditionalLockRelationForExtension functions and we can reset it in > > UnlockRelationForExtension or in the error path e.g. LockReleaseAll. > > > > I think if we reset it in LockReleaseAll during the error path, then > we need to find a way to reset it during LockReleaseCurrentOwner as > that is called during Subtransaction Abort which can be tricky as we > don't know if it belongs to the current owner. How about resetting in > Abort(Sub)Transaction and CommitTransaction after we release locks via > ResourceOwnerRelease.
I think instead of the flag we need to keep the counter because we can acquire the same relation extension lock multiple times. So basically, every time we acquire the lock we can increment the counter and while releasing we can decrement it. During an error path, I think it is fine to set it to 0 in CommitTransaction/AbortTransaction. But, I am not sure that we can set to 0 or decrement it in AbortSubTransaction because we are not sure whether we have acquired the lock under this subtransaction or not.
Having said that, I think there should not be any case that we are starting the sub-transaction while holding the relation extension lock.
Right, this is exactly the point. I think we can mention this in comments to make it clear why setting it to zero is fine during subtransaction abort.
Is there anything wrong with having an Assert during subtransaction start to indicate that we don't have a relation extension lock?
