From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Steve Singer <steve@ssinger.info>
Cc: pgsql-hackers@lists.postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: 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: Tue, 1 Sep 2020 05:12:19 -0500
Message-ID: <20200901101219.GB5450@telsasoft.com> (raw)
In-Reply-To: <20200811070922.GP20473@telsasoft.com>
References: <a0fd6af4bdf3cd209a69cea112ab10ea@postgrespro.ru>
<20200406184406.GF2228@telsasoft.com>
<9b6a6cec134451550b74aa2ffd1446d0@postgrespro.ru>
<20200407204406.GR2228@telsasoft.com>
<20200412013352.GA2491@telsasoft.com>
<20200426175614.GT28974@telsasoft.com>
<20200809110252.GD17986@paquier.xyz>
<20200810022443.GI20473@telsasoft.com>
<20200811053945.GG17986@paquier.xyz>
<20200811070922.GP20473@telsasoft.com>
This patch seems to be missing a call to RelationAssumeNewRelfilenode() in
reindex_index().
That's maybe the related to the cause of the crashes I pointed out earlier this
year.
Alexey's v4 patch changed RelationSetNewRelfilenode() to accept a tablespace
parameter, but Michael seemed to object to that. However that seems cleaner
and ~30 line shorter.
Michael, would you comment on that ? The v4 patch and your comments are here.
https://www.postgresql.org/message-id/attachment/105574/v4-0001-Allow-REINDEX-and-REINDEX-CONCURRENT...https://www.postgresql.org/message-id/20191127035416.GG5435%40paquier.xyz> --- a/src/backend/catalog/index.c
> +++ b/src/backend/catalog/index.c
> @@ -3480,6 +3518,47 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
> */
> CheckTableNotInUse(iRel, "REINDEX INDEX");
>
> + if (tablespaceOid == MyDatabaseTableSpace)
> + tablespaceOid = InvalidOid;
> +
> + /*
> + * Set the new tablespace for the relation. Do that only in the
> + * case where the reindex caller wishes to enforce a new tablespace.
> + */
> + if (set_tablespace &&
> + tablespaceOid != iRel->rd_rel->reltablespace)
> + {
> + Relation pg_class;
> + Form_pg_class rd_rel;
> + HeapTuple tuple;
> +
> + /* First get a modifiable copy of the relation's pg_class row */
> + pg_class = table_open(RelationRelationId, RowExclusiveLock);
> +
> + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
> + if (!HeapTupleIsValid(tuple))
> + elog(ERROR, "cache lookup failed for relation %u", indexId);
> + rd_rel = (Form_pg_class) GETSTRUCT(tuple);
> +
> + /*
> + * Mark the relation as ready to be dropped at transaction commit,
> + * before making visible the new tablespace change so as this won't
> + * miss things.
> + */
> + RelationDropStorage(iRel);
> +
> + /* Update the pg_class row */
> + rd_rel->reltablespace = tablespaceOid;
> + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
> +
> + heap_freetuple(tuple);
> +
> + table_close(pg_class, RowExclusiveLock);
> +
> + /* Make sure the reltablespace change is visible */
> + CommandCounterIncrement();
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, masahiko.sawada@2ndquadrant.com, steve@ssinger.info, alvherre@2ndquadrant.com, 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: <20200901101219.GB5450@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 DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox