From: Álvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
Date: Thu, 6 May 2021 13:13:47 -0400
Message-ID: <20210506171347.GA27028@alvherre.pgsql> (raw)
In-Reply-To: <CA+HiwqE7GxGU4VdzwZzfiz+Ont5SsopoFkgtrZGEdPqWRL+biA@mail.gmail.com>
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.
> 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.
--
Álvaro Herrera Valdivia, Chile
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, amitlangote09@gmail.com, pryzby@telsasoft.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
In-Reply-To: <20210506171347.GA27028@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox