Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Justin Pryzby <pryzby@telsasoft.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Steve Singer <steve@ssinger.info>, pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, Masahiko Sawada <sawada.mshk@gmail.com>, Jose Luis Tallon <jltallon@adv-solutions.net>
Date: 2021-02-02T01:32:19Z
Lists: pgsql-hackers
On Mon, Feb 01, 2021 at 06:28:57PM +0300, Alexey Kondratov wrote:
> Hm, IIUC, REINDEX CONCURRENTLY doesn't use either of them. It directly uses
> index_create() with a proper tablespaceOid instead of
> SetRelationTableSpace(). And its checks structure is more restrictive even
> without tablespace change, so it doesn't use CheckRelationTableSpaceMove().

Sure.  I have not checked the patch in details, but even with that it
would be much safer to me if we apply the same sanity checks
everywhere.  That's less potential holes to worry about.

> Basically, it implements this option "we could silently *not* do the switch
> for partitioned indexes in the flow of REINDEX, letting users handle that
> with an extra ALTER TABLE SET TABLESPACE once REINDEX has finished". It
> probably makes sense, since we are doing tablespace change altogether with
> index relation rewrite and don't touch relations without storage. Doing
> ALTER INDEX ... SET TABLESPACE will be almost cost-less on them, since they
> do not hold any data.

Yeah, they'd still need an AEL for a short time on the partitioned
bits with what's on HEAD.  I'll keep in mind to look at the
possibility to downgrade this lock if cascading only on partitioned
tables.  The main take is that AlterTableGetLockLevel() cannot select
a lock type based on the table meta-data.  Tricky problem it is if
taken as a whole, but I guess that we should be able to tweak ALTER
TABLE ONLY on a partitioned table/index pretty easily (inh = false).
--
Michael

Commits

  1. Add TABLESPACE option to REINDEX

  2. Refactor code in tablecmds.c to check and process tablespace moves

  3. Refactor option handling of CLUSTER, REINDEX and VACUUM

  4. pg_dump: Don't use enums for defining bit mask values

  5. Refactor CLUSTER and REINDEX grammar to use DefElem for option lists

  6. Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE

  7. Improve tab completion of REINDEX in psql

  8. Fix possible crash during FATAL exit from reindexing.