Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager

On Sat, Mar 7, 2020 at 11:14 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> On Sat, Mar 7, 2020 at 9:57 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
>> On Fri, Mar 6, 2020 at 9:47 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
>> wrote:
>> >
>> > On Thu, Mar 5, 2020 at 1:54 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com>
>> wrote:
>> > >
>> > > On Thu, Mar 5, 2020 at 12:15 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com