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

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Steve Singer <steve(at)ssinger(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Jose Luis Tallon <jltallon(at)adv-solutions(dot)net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: 2021-01-29 17:56:47
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2021-01-28 14:42, Alexey Kondratov wrote:
> On 2021-01-28 00:36, Alvaro Herrera wrote:

>> I didn't look at the patch closely enough to understand why you're
>> trying to do something like CLUSTER, VACUUM FULL or REINDEX without
>> holding full AccessExclusiveLock on the relation. But do keep in mind
>> that once you hold a lock on a relation, trying to grab a weaker lock
>> afterwards is pretty pointless.
> No, you are right, we are doing REINDEX with AccessExclusiveLock as it
> was before. This part is a more specific one. It only applies to
> partitioned indexes, which do not hold any data, so we do not reindex
> them directly, only their leafs. However, if we are doing a TABLESPACE
> change, we have to record it in their pg_class entry, so all future
> leaf partitions were created in the proper tablespace.
> That way, we open partitioned index relation only for a reference,
> i.e. read-only, but modify pg_class entry under a proper lock
> (RowExclusiveLock). That's why I thought that ShareLock will be
> enough.
> IIUC, 'ALTER TABLE ... SET TABLESPACE' uses AccessExclusiveLock even
> for relations with no storage, since AlterTableGetLockLevel() chooses
> it if AT_SetTableSpace is met. This is very similar to our case, so
> probably we should do the same?
> Actually it is not completely clear for me why
> ShareUpdateExclusiveLock is sufficient for newly added
> SetRelationTableSpace() as Michael wrote in the comment.

Changed patch to use AccessExclusiveLock in this part for now. This is
Anyway, all real leaf partitions are processed in the independent
transactions later.

Also changed some doc/comment parts Justin pointed me to.

>> + then all "mapped" and system relations will be skipped and a
>> single
>> + <literal>WARNING</literal> will be generated. Indexes on TOAST
>> tables
>> + are reindexed, but not moved the new tablespace.
> moved *to* the new tablespace.


> I don't know if that needs to be said at all. We talked about it a lot
> to
> arrive at the current behavior, but I think that's only due to the
> difficulty
> of correcting the initial mistake.

I do not think that it will be a big deal to move indexes on TOAST
tables as well. I just thought that since 'ALTER TABLE/INDEX ... SET
TABLESPACE' only moves them together with host table, we also should not
do that. Yet, I am ready to change this logic if requested.

Alexey Kondratov

Postgres Professional
Russian Postgres Company

Attachment Content-Type Size
v9-0001-Allow-REINDEX-to-change-tablespace.patch text/x-diff 31.9 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2021-01-29 18:43:44 Re: Add MAIN_RELATION_CLEANUP and SECONDARY_RELATION_CLEANUP options to VACUUM
Previous Message Antonin Houska 2021-01-29 17:30:15 Re: POC: Cleaning up orphaned files using undo logs