Re: ATTACH/DETACH PARTITION CONCURRENTLY

On Fri, Dec 21, 2018 at 6:04 PM David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On Fri, 21 Dec 2018 at 09:43, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > - I refactored expand_inherited_rtentry() to drive partition expansion
> > entirely off of PartitionDescs. The reason why this is necessary is
> > that it clearly will not work to have find_all_inheritors() use a
> > current snapshot to decide what children we have and lock them, and
> > then consult a different source of truth to decide which relations to
> > open with NoLock. There's nothing to keep the lists of partitions
> > from being different in the two cases, and that demonstrably causes
> > assertion failures if you SELECT with an ATTACH/DETACH loop running in
> > the background. However, it also changes the order in which tables get
> > locked. Possibly that could be fixed by teaching
> > expand_partitioned_rtentry() to qsort() the OIDs the way
> > find_inheritance_children() does. It also loses the infinite-loop
> > protection which find_all_inheritors() has. Not sure what to do about
> > that.
>
> I don't think you need to qsort() the Oids before locking. What the
> qsort() does today is ensure we get a consistent locking order. Any
> other order would surely do, providing we stick to it consistently. I
> think PartitionDesc order is fine, as it's consistent. Having it
> locked in PartitionDesc order I think is what's needed for [1] anyway.
> [2] proposes to relax the locking order taken during execution.
>
> [1] https://commitfest.postgresql.org/21/1778/
> [2] https://commitfest.postgresql.org/21/1887/

Based on this feedback, I went ahead and committed the part of the
previously-posted patch set that makes this change.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company