|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
> 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
what 'ALTER TABLE/INDEX ... SET TABLESPACE' and 'REINDEX' usually do.
Anyway, all real leaf partitions are processed in the independent
Also changed some doc/comment parts Justin pointed me to.
>> + then all "mapped" and system relations will be skipped and a
>> + <literal>WARNING</literal> will be generated. Indexes on TOAST
>> + 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
> arrive at the current behavior, but I think that's only due to the
> 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.
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
|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|