Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

On Fri, May 7, 2021 at 2:13 AM Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> On 2021-Apr-30, Amit Langote wrote:
>
> > The case I was looking at is when a partition detach appears as
> > in-progress to a serializable transaction.
>
> Yeah, I was exceedingly sloppy on my reasoning about this, and you're
> right that that's what actually happens rather than what I said.
>
> > If the caller wants to omit detached partitions, such a partition ends
> > up in rd_partdesc_nodetached, with the corresponding xmin being set to
> > 0 due to the way find_inheritance_children_extended() sets
> > *detached_xmin. The next query in the transaction that wants to omit
> > detached partitions, seeing rd_partdesc_nodetached_xmin being invalid,
> > rebuilds the partdesc, again including that partition because the
> > snapshot wouldn't have changed, and so on until the transaction ends.
> > Now, this can perhaps be "fixed" by making
> > find_inheritance_children_extended() set the xmin outside the
> > snapshot-checking block, but maybe there's no need to address this on
> > priority.
>
> Hmm. See below.
>
> > Rather, a point that bothers me a bit is that we're including a
> > detached partition in the partdesc labeled "nodetached" in this
> > particular case. Maybe we should avoid that by considering in this
> > scenario that no detached partitions exist for this transactions and
> > so initialize rd_partdesc, instead of rd_partdesc_nodetached. That
> > will let us avoid the situations where the xmin is left in invalid
> > state. Maybe like the attached (it also fixes a couple of
> > typos/thinkos in the previous commit).
>
> Makes sense -- applied, thanks.

Thank you.

> > Note that we still end up in the same situation as before where each
> > query in the serializable transaction that sees the detach as
> > in-progress to have to rebuild the partition descriptor omitting the
> > detached partitions, even when it's clear that the detach-in-progress
> > partition will be included every time.
>
> Yeah, you're right that there is a performance hole in the case where a
> partition pending detach exists and you're using repeatable read
> transactions. I didn't see it as terribly critical since it's supposed
> to be very transient, but I may be wrong.

Yeah, I'd hope so too. I think RR transactions would have to be
concurrent with an interrupted DETACH CONCURRENTLY to suffer the
performance hit and that does kind of make this a rarely occurring
case.

--
Amit Langote
EDB: http://www.enterprisedb.com