Re: pg_restore causing deadlocks on partitioned tables

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Domagoj Smoljanovic <domagoj(dot)smoljanovic(at)oradian(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_restore causing deadlocks on partitioned tables
Date: 2020-09-14 23:07:21
Message-ID: 20200914230721.GA9788@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020-Sep-14, Tom Lane wrote:

> > (1) TRUNCATE on a partition tries to get AccessShareLock on the parent;
>
> The reason for this is that
>
> (a) ExecuteTruncateGuts calls InitResultRelInfo, because it might
> need that to fire TRUNCATE triggers for the child relation.

Hmm, this seems legitimate, but of course we don't need the partition
qual. So the reported bug would be solved with just the change to avoid
loading ri_PartitionExpr until needed.

> AFAICS, it is utterly silly for InitResultRelInfo to be forcing
> a partition qual to be computed when we might not need it.
> We could flush ResultRelInfo.ri_PartitionCheck altogether and
> have anything that was reading it instead do
> RelationGetPartitionQual(ResultRelInfo.ri_RelationDesc).

Hmm, but I presume we don't want to compute it every time. I suggest we
would still have it, but we'd only computed it when first used.

> Actually it looks like most of the places reading it are
> just interested in non-nullness; can't those be nuked from
> orbit in favor of testing rel->rd_rel->relispartition?
> There's no such thing as a partition with an empty partition
> qual is there? (Or even if it's possible, do we care about
> optimizing the case?)

Actually, there is one such case -- when the default partition is the
only partition, its constraint is empty. This has caused at least one
bug. Maybe it'd be better if we used something like constant true
instead ... since we're not likely to care much about the performance of
that case. But I don't think that would change this patch any.

> > (2) ALTER TABLE ONLY ... ADD CONSTRAINT on a partition root tries to get
> > AccessExclusiveLock on all child partitions, despite the ONLY.
>
> The cause of this seems to be that ATPrepSetNotNull is too dumb to
> avoid recursing to all the child tables when the parent is already
> attnotnull. Or is there a reason we have to recurse anyway?

Hmm, looking at ATExecSetNotNull, we invoke the PostAlter hook even when
there's no change, so if we supressed the recursion early, that would
change. But I doubt we actually care.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2020-09-14 23:17:18 Re: Improving connection scalability: GetSnapshotData()
Previous Message Peter Geoghegan 2020-09-14 23:03:14 Re: logtape.c stats don't account for unused "prefetched" block numbers