Horton

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

Dilip Kumar <dilipbalaut@gmail.com>

View thread
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Mahendra Singh Thalor <mahi6run@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Michael Paquier <michael@paquier.xyz>, Mithun Cy <mithun.cy@enterprisedb.com>, Thomas Munro <thomas.munro@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-03-07T14:24:11Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow page lock to conflict among parallel group members.

  2. Allow relation extension lock to conflict among parallel group members.

  3. Add assert to ensure that page locks don't participate in deadlock cycle.

  4. Assert that we don't acquire a heavyweight lock on another object after

  5. Fix unsafe usage of strerror(errno) within ereport().

On Sat, Mar 7, 2020 at 3:26 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Mar 7, 2020 at 11:14 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Sat, Mar 7, 2020 at 9:57 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>>
>>> 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?

Yes, I was planning to do that.

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