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: Robert Haas <robertmhaas@gmail.com>, Mahendra Singh Thalor <mahi6run@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, 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-16T05:13:35Z
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 Mon, Mar 16, 2020 at 8:15 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sun, Mar 15, 2020 at 9:17 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Sun, Mar 15, 2020 at 5:58 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > >
> > > 1. Group members wait for page locks.  If you test that the leader
> > > acquires the page lock and then member also tries to acquire the same
> > > lock on the same index, it wouldn't block before the patch, but after
> > > the patch, the member should wait for the leader to release the lock.
> >
> > Okay, I will test this part.
> >
> > > 2. Try to hit Assert in LockAcquireExtended (a) by trying to
> > > re-acquire the page lock via the debugger,
> >
> > I am not sure whether it is true or not,  Because, if we are holding
> > the page lock and we try the same page lock then the lock will be
> > granted without reaching the code path.  However, I agree that this is
> > not intended instead this is a side effect of allowing relation
> > extension lock while holding the same relation extension lock.  So
> > basically, now the situation is that if the lock is directly granted
> > because we are holding the same lock then it will not go to the assert
> > code.  IMHO, we don't need to add extra code to make it behave
> > differently.  Please let me know what is your opinion on this.
> >
>
> I also don't think there is any reason to add code to prevent that.
> Actually, what I wanted to test was to somehow hit the Assert for the
> cases where it will actually hit if someone tomorrow tries to acquire
> any other type of lock.  Can we mimic such a situation by hacking code
> (say try to acquire some other type of heavyweight lock) or in some
> way to hit the newly added Assert?

I have hacked the code by calling another heavyweight lock and the
assert is hit.

>
> > (b) try to acquire the
> > > relation extension lock after page lock and it should be allowed
> > > (after acquiring page lock, we take relation extension lock in
> > > following code path:
> > > ginInsertCleanup->ginEntryInsert->ginFindLeafPage->ginPlaceToPage->GinNewBuffer).

I have tested this part and it works as expected i.e. assert is not hit.

--test case
create table gin_test_tbl(i int4[]) with (autovacuum_enabled = off);
create index gin_test_idx on gin_test_tbl using gin (i);
insert into gin_test_tbl select array[1, 2, g] from generate_series(1, 20000) g;
select gin_clean_pending_list('gin_test_idx');

BTW, this test is already covered by the existing gin.sql file so we
don't need to add any new test.

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