Re: Delay locking partitions during INSERT and UPDATE

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, John Naylor <john(dot)naylor(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Delay locking partitions during INSERT and UPDATE
Date: 2019-02-18 23:28:32
Message-ID: CAKJS1f--dYPNX_gRr19_0tuaVyCbZdACvk4S4SNjiix3cuhahQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 19 Feb 2019 at 12:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Now that I've looked at *this* thread's patch, I agree that having
> the executor acquire locks based on find_all_inheritors() is a
> pretty horrid idea. However, it still seems like this is being
> done in a vacuum without attention to locks already acquired
> upstream. How much does the knowledge that the planner or plancache
> would've already locked everything mentioned in the rangetable
> affect the issues here?

For the INSERT with VALUES case, there should be no existing locks on
the partition. Only the partitioned table that's the target of the
INSERT will have been locked already.

For the INSERT with SELECT case, well, it's possible that one of the
partitions exist in the SELECT and it was already locked by the
planner or in AcquireExecutorLocks(), but how is that different from
having multiple RangeTblEntry objects for the same relation? We
already try to lock those twice. If we had code to resolve those
duplicates and take just the strongest of the locks for the relation
then I might feel we'd better handle the locking better in this patch,
but we don't, so I see no reason to add additional smarts for a corner
case if we have none for a more common case.

For the UPDATE case, we reuse result relations from the ModifyTable's
subnodes. See ExecFindPartition() around:

/*
* We have not yet set up a ResultRelInfo for this partition,
* but if we have a subplan hash table, we might have one
* there. If not, we'll have to create one.
*/
if (proute->subplan_resultrel_htab)

> I'm inclined to think that if we already have lock on the parent
> partitioned table (thereby, IIUC, guaranteeing that its partitioning
> info can't change) that the order in which we acquire the same lock
> level on its partition(s) isn't very important.

Yeah. I think most people seem to agree with that too. At least,
nobody seemed to raise any concerns about increased deadlock risk,
although it was talked about.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2019-02-18 23:30:17 Re: Delay locking partitions during INSERT and UPDATE
Previous Message Tom Lane 2019-02-18 23:23:16 Re: 2019-03 CF Summary / Review - Tranche #2