Re: Assert while autovacuum was executing

On Fri, Jun 23, 2023 at 2:04 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Jun 22, 2023 at 9:16 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Jun 21, 2023 at 10:57 AM Andres Freund <andres@anarazel.de> wrote:
> > >
> > > As far as I can tell 72e78d831a as-is is just bogus. Unfortunately that likely
> > > also means 3ba59ccc89 is not right.
> > >
> >
> > Indeed. I was thinking of a fix but couldn't find one yet. One idea I
> > am considering is to allow catalog table locks after page lock but I
> > think apart from hacky that also won't work because we still need to
> > remove the check added for page locks in the deadlock code path in
> > commit 3ba59ccc89 and may need to do something for group locking.
> >
>
> I have further thought about this part and I think even if we remove
> the changes in commit 72e78d831a (remove the assertion for page locks
> in LockAcquireExtended()) and remove the check added for page locks in
> FindLockCycleRecurseMember() via commit 3ba59ccc89, it is still okay
> to keep the change related to "allow page lock to conflict among
> parallel group members" in LockCheckConflicts(). This is because locks
> on catalog tables don't conflict among group members. So, we shouldn't
> see a deadlock among parallel group members. Let me try to explain
> this thought via an example:
>

IMHO, whatsoever the case this check[1], is not wrong at all.  I agree
that we do not have parallel write present in the code so having this
check is not necessary as of now.  But in theory, this check is
correct because this is saying that parallel leader and worker should
conflict on the 'relation extension lock' and the 'page lock' and
that's the fact.  It holds true irrespective of whether it is being
used currently or not.


[1]
/*
* The relation extension or page lock conflict even between the group
* members.
*/
if (LOCK_LOCKTAG(*lock) == LOCKTAG_RELATION_EXTEND ||
(LOCK_LOCKTAG(*lock) == LOCKTAG_PAGE))
{
PROCLOCK_PRINT("LockCheckConflicts: conflicting (group)",
   proclock);
return true;
}


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