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

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager
Date: 2020-03-07 14:24:11
Message-ID: CAFiTN-sji0F7HGh12x8ZMpuTgC8u4MooLC_WqLN=cViYear9Sg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Mar 7, 2020 at 3:26 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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?

Yes, I was planning to do that.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ivan Panchenko 2020-03-07 15:07:24 Re[6]: bool_plperl transform
Previous Message Fabien COELHO 2020-03-07 14:14:37 Re: pg_ls_tmpdir to show directories and shared filesets