agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Steve Singer <steve@ssinger.info>
Cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Jose Luis Tallon <jltallon@adv-solutions.net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Date: Sun, 14 Feb 2021 20:10:50 -0600
Message-ID: <20210215021050.GA28165@telsasoft.com> (raw)
In-Reply-To: <YBuWbzoW6W7AaMv0@paquier.xyz>
References: <20210127213658.GA736@alvherre.pgsql>
<55c6c1526b94d2caf54c33262fe00674@postgrespro.ru>
<e15777decc533006c9dbe4988bd66d31@postgrespro.ru>
<YBTC0IYIGmyYfiIn@paquier.xyz>
<1c506564e2a732ec17790bd8c34f042e@postgrespro.ru>
<YBiro+n0767QDXSA@paquier.xyz>
<YBpEsxZxIVn0zyFM@paquier.xyz>
<20210203065342.GX7450@telsasoft.com>
<YBqA8snxOp2lXLyl@paquier.xyz>
<YBuWbzoW6W7AaMv0@paquier.xyz>
On Thu, Feb 04, 2021 at 03:38:39PM +0900, Michael Paquier wrote:
> On Wed, Feb 03, 2021 at 07:54:42PM +0900, Michael Paquier wrote:
> > Not sure I like that. Here is a proposal:
> > "it is recommended to separately use ALTER TABLE ONLY on them so as
> > any new partitions attached inherit the new tablespace value."
>
> So, I have done more work on this stuff today, and applied that as of
> c5b2860.
> A second thing I have come back to is allow_system_table_mods for
> toast relations, and decided to just forbid TABLESPACE if attempting
> to use it directly on a system table even if allow_system_table_mods
> is true. This was leading to inconsistent behaviors and weirdness in
> the concurrent case because all the indexes are processed in series
> after building a list. As we want to ignore the move of toast indexes
> when moving the indexes of the parent table, this was leading to extra
> conditions that are not really worth supporting after thinking about
> it. One other issue was the lack of consistency when using pg_global
> that was a no-op for the concurrent case but failed in the
> non-concurrent case. I have put in place more regression tests for
> all that.
Isn't this dead code ?
postgres=# REINDEX (CONCURRENTLY, TABLESPACE pg_global) TABLE pg_class;
ERROR: 0A000: cannot reindex system catalogs concurrently
LOCATION: ReindexRelationConcurrently, indexcmds.c:3276
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 127ba7835d..c77a9b2563 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3260,73 +3260,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
{
if (IsCatalogRelationOid(relationOid))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
...
- if (OidIsValid(params->tablespaceOid) &&
- IsSystemRelation(heapRelation))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot move system relation \"%s\"",
- RelationGetRelationName(heapRelation))));
-
@@ -3404,73 +3397,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
if (IsCatalogRelationOid(heapId))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
...
- if (OidIsValid(params->tablespaceOid) &&
- IsSystemRelation(heapRelation))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot move system relation \"%s\"",
- get_rel_name(relationOid))));
-
Attachments:
[text/x-diff] 0001-Dead-code-REINDEX-CONCURRENTLY-TABLESPACE-.-c5b28604.patch (5.5K, ../20210215021050.GA28165@telsasoft.com/2-0001-Dead-code-REINDEX-CONCURRENTLY-TABLESPACE-.-c5b28604.patch)
download | inline diff:
From b4347c18bc732d30295c065ef71edaac65e68fe6 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 14 Feb 2021 19:49:52 -0600
Subject: [PATCH] Dead code: REINDEX (CONCURRENTLY, TABLESPACE ..):
c5b286047cd698021e57a527215b48865fd4ad4e
---
src/backend/commands/indexcmds.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 127ba7835d..c77a9b2563 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3260,73 +3260,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
{
/*
* In the case of a relation, find all its indexes including
* toast indexes.
*/
Relation heapRelation;
/* Save the list of relation OIDs in private context */
oldcontext = MemoryContextSwitchTo(private_context);
/* Track this relation for session locks */
heapRelationIds = lappend_oid(heapRelationIds, relationOid);
MemoryContextSwitchTo(oldcontext);
if (IsCatalogRelationOid(relationOid))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
/* Open relation to get its indexes */
if ((params->options & REINDEXOPT_MISSING_OK) != 0)
{
heapRelation = try_table_open(relationOid,
ShareUpdateExclusiveLock);
/* leave if relation does not exist */
if (!heapRelation)
break;
}
else
heapRelation = table_open(relationOid,
ShareUpdateExclusiveLock);
- if (OidIsValid(params->tablespaceOid) &&
- IsSystemRelation(heapRelation))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot move system relation \"%s\"",
- RelationGetRelationName(heapRelation))));
-
/* Add all the valid indexes of relation to list */
foreach(lc, RelationGetIndexList(heapRelation))
{
Oid cellOid = lfirst_oid(lc);
Relation indexRelation = index_open(cellOid,
ShareUpdateExclusiveLock);
if (!indexRelation->rd_index->indisvalid)
ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex invalid index \"%s.%s\" concurrently, skipping",
get_namespace_name(get_rel_namespace(cellOid)),
get_rel_name(cellOid))));
else if (indexRelation->rd_index->indisexclusion)
ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex exclusion constraint index \"%s.%s\" concurrently, skipping",
get_namespace_name(get_rel_namespace(cellOid)),
get_rel_name(cellOid))));
else
{
ReindexIndexInfo *idx;
/* Save the list of relation OIDs in private context */
oldcontext = MemoryContextSwitchTo(private_context);
idx = palloc(sizeof(ReindexIndexInfo));
idx->indexId = cellOid;
/* other fields set later */
indexIds = lappend(indexIds, idx);
MemoryContextSwitchTo(oldcontext);
@@ -3404,73 +3397,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
/*
* Don't allow reindex for an invalid index on TOAST table, as
* if rebuilt it would not be possible to drop it. Match
* error message in reindex_index().
*/
if (IsToastNamespace(get_rel_namespace(relationOid)) &&
!get_index_isvalid(relationOid))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex invalid index on TOAST table")));
/*
* Check if parent relation can be locked and if it exists,
* this needs to be done at this stage as the list of indexes
* to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
* should not be used once all the session locks are taken.
*/
if ((params->options & REINDEXOPT_MISSING_OK) != 0)
{
heapRelation = try_table_open(heapId,
ShareUpdateExclusiveLock);
/* leave if relation does not exist */
if (!heapRelation)
break;
}
else
heapRelation = table_open(heapId,
ShareUpdateExclusiveLock);
- if (OidIsValid(params->tablespaceOid) &&
- IsSystemRelation(heapRelation))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot move system relation \"%s\"",
- get_rel_name(relationOid))));
-
table_close(heapRelation, NoLock);
/* Save the list of relation OIDs in private context */
oldcontext = MemoryContextSwitchTo(private_context);
/* Track the heap relation of this index for session locks */
heapRelationIds = list_make1_oid(heapId);
/*
* Save the list of relation OIDs in private context. Note
* that invalid indexes are allowed here.
*/
idx = palloc(sizeof(ReindexIndexInfo));
idx->indexId = relationOid;
indexIds = lappend(indexIds, idx);
/* other fields set later */
MemoryContextSwitchTo(oldcontext);
break;
}
case RELKIND_PARTITIONED_TABLE:
case RELKIND_PARTITIONED_INDEX:
default:
/* Return error if type of relation is not supported */
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot reindex this type of relation concurrently")));
break;
}
/*
* Definitely no indexes, so leave. Any checks based on
--
2.17.0
view thread (154+ messages) latest in thread
Message-ID: <20210215021050.GA28165@telsasoft.com>
Permalink: ../20210215021050.GA28165@telsasoft.com/
Also on: postgresql.org/message-id/20210215021050.GA28165@telsasoft.com
reply
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: pryzby@telsasoft.com, michael@paquier.xyz, a.kondratov@postgrespro.ru, alvherre@alvh.no-ip.org, peter.eisentraut@enterprisedb.com, masahiko.sawada@2ndquadrant.com, steve@ssinger.info, robertmhaas@gmail.com, a.korotkov@postgrespro.ru, sawada.mshk@gmail.com, jltallon@adv-solutions.net
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
In-Reply-To: <20210215021050.GA28165@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox