On Thu, Apr 18, 2019 at 10:14:30AM +0900, Michael Paquier wrote: > Doing a REINDEX TABLE directly on pg_class proves to work correctly, > and CONCURRENTLY is not supported for catalog tables. > > Bisecting my way through it, the first commit causing the breakage is > that: > commit: 01e386a325549b7755739f31308de4be8eea110d > author: Tom Lane <t...@sss.pgh.pa.us> > date: Wed, 23 Dec 2015 20:09:01 -0500 > Avoid VACUUM FULL altogether in initdb.
This brings down to a first, simple, solution which is to issue a
VACUUM FULL on pg_class at the end of make_template0() in initdb.c to
avoid any subsequent problems if trying to issue a REINDEX on anything
related to pg_class, and it won't fix any existing deployments:
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2042,6 +2042,11 @@ make_template0(FILE *cmdfd)
* Finally vacuum to clean up dead rows in pg_database
*/
"VACUUM pg_database;\n\n",
+
+ /*
+ * And rebuild pg_class.
+ */
+ "VACUUM FULL pg_class;\n\n",
NULL
};
Now...
> The reason why this does not work is that CatalogIndexInsert() tries
> to do an index_insert directly on the index worked on. And the reason
> why this works at table level is that we have tweaks in
> reindex_relation() to enforce the list of valid indexes in the
> relation cache with RelationSetIndexList(). It seems to me that the
> logic in reindex_index() is wrong from the start, and that all the
> index list handling done in reindex_relation() should just be in
> reindex_index() so as REINDEX INDEX gets the right call.
I got to wonder if this dance with the relation cache is actually
necessary, because we could directly tell CatalogIndexInsert() to not
insert a tuple into an index which is bring rebuilt, and the index
rebuild would cause an entry to be added to pg_class anyway thanks to
RelationSetNewRelfilenode(). This can obviously only happen for
pg_class indexes.
Any thoughts about both approaches?
--
Michael
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index e9399bef14..d4284ba08a 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3502,7 +3502,6 @@ reindex_relation(Oid relid, int flags, int options)
Relation rel;
Oid toast_relid;
List *indexIds;
- bool is_pg_class;
bool result;
int i;
@@ -3538,32 +3537,8 @@ reindex_relation(Oid relid, int flags, int options)
*/
indexIds = RelationGetIndexList(rel);
- /*
- * reindex_index will attempt to update the pg_class rows for the relation
- * and index. If we are processing pg_class itself, we want to make sure
- * that the updates do not try to insert index entries into indexes we
- * have not processed yet. (When we are trying to recover from corrupted
- * indexes, that could easily cause a crash.) We can accomplish this
- * because CatalogTupleInsert/CatalogTupleUpdate will use the relcache's
- * index list to know which indexes to update. We just force the index
- * list to be only the stuff we've processed.
- *
- * It is okay to not insert entries into the indexes we have not processed
- * yet because all of this is transaction-safe. If we fail partway
- * through, the updated rows are dead and it doesn't matter whether they
- * have index entries. Also, a new pg_class index will be created with a
- * correct entry for its own pg_class row because we do
- * RelationSetNewRelfilenode() before we do index_build().
- */
- is_pg_class = (RelationGetRelid(rel) == RelationRelationId);
-
- /* Ensure rd_indexattr is valid; see comments for RelationSetIndexList */
- if (is_pg_class)
- (void) RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_ALL);
-
PG_TRY();
{
- List *doneIndexes;
ListCell *indexId;
char persistence;
@@ -3591,15 +3566,11 @@ reindex_relation(Oid relid, int flags, int options)
persistence = rel->rd_rel->relpersistence;
/* Reindex all the indexes. */
- doneIndexes = NIL;
i = 1;
foreach(indexId, indexIds)
{
Oid indexOid = lfirst_oid(indexId);
- if (is_pg_class)
- RelationSetIndexList(rel, doneIndexes);
-
reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
persistence, options);
@@ -3608,9 +3579,6 @@ reindex_relation(Oid relid, int flags, int options)
/* Index should no longer be in the pending list */
Assert(!ReindexIsProcessingIndex(indexOid));
- if (is_pg_class)
- doneIndexes = lappend_oid(doneIndexes, indexOid);
-
/* Set index rebuild count */
pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT,
i);
@@ -3626,9 +3594,6 @@ reindex_relation(Oid relid, int flags, int options)
PG_END_TRY();
ResetReindexPending();
- if (is_pg_class)
- RelationSetIndexList(rel, indexIds);
-
/*
* Close rel, but continue to hold the lock.
*/
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 0c994122d8..d102fcecae 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -114,6 +114,22 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
if (!indexInfo->ii_ReadyForInserts)
continue;
+ /*
+ * If trying to insert a tuple for an index which is being rebuilt,
+ * ignore it. This can happen only for an index related to pg_class.
+ *
+ * A REINDEX being transaction-safe, it is fine to fail partway as the
+ * inserted rows are dead and it doesn't matter whether they have index
+ * entries. A new pg_class index will be created with a correct entry
+ * for its own pg_class row because we do RelationSetNewRelfilenode()
+ * before we do index_build() in reindex_index().
+ */
+ if (ReindexIsProcessingIndex(RelationGetRelid(relationDescs[i])))
+ {
+ Assert(RelationGetRelid(heapRelation) == RelationRelationId);
+ continue;
+ }
+
/*
* Expressional and partial indexes on system catalogs are not
* supported, nor exclusion constraints, nor deferred uniqueness
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index bab59f16e6..e5cb9eb916 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4544,52 +4544,6 @@ insert_ordered_oid(List *list, Oid datum)
return list;
}
-/*
- * RelationSetIndexList -- externally force the index list contents
- *
- * This is used to temporarily override what we think the set of valid
- * indexes is (including the presence or absence of an OID index).
- * The forcing will be valid only until transaction commit or abort.
- *
- * This should only be applied to nailed relations, because in a non-nailed
- * relation the hacked index list could be lost at any time due to SI
- * messages. In practice it is only used on pg_class (see REINDEX).
- *
- * It is up to the caller to make sure the given list is correctly ordered.
- *
- * We deliberately do not change rd_indexattr here: even when operating
- * with a temporary partial index list, HOT-update decisions must be made
- * correctly with respect to the full index set. It is up to the caller
- * to ensure that a correct rd_indexattr set has been cached before first
- * calling RelationSetIndexList; else a subsequent inquiry might cause a
- * wrong rd_indexattr set to get computed and cached. Likewise, we do not
- * touch rd_keyattr, rd_pkattr or rd_idattr.
- */
-void
-RelationSetIndexList(Relation relation, List *indexIds)
-{
- MemoryContext oldcxt;
-
- Assert(relation->rd_isnailed);
- /* Copy the list into the cache context (could fail for lack of mem) */
- oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
- indexIds = list_copy(indexIds);
- MemoryContextSwitchTo(oldcxt);
- /* Okay to replace old list */
- list_free(relation->rd_indexlist);
- relation->rd_indexlist = indexIds;
-
- /*
- * For the moment, assume the target rel hasn't got a pk or replica index.
- * We'll load them on demand in the API that wraps access to them.
- */
- relation->rd_pkindex = InvalidOid;
- relation->rd_replidindex = InvalidOid;
- relation->rd_indexvalid = 2; /* mark list as forced */
- /* Flag relation as needing eoxact cleanup (to reset the list) */
- EOXactListAdd(relation);
-}
-
/*
* RelationGetPrimaryKeyIndex -- get OID of the relation's primary key index
*
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 809d6aa123..364495a5f0 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -66,9 +66,6 @@ extern void RelationGetExclusionInfo(Relation indexRelation,
Oid **procs,
uint16 **strategies);
-extern void RelationSetIndexList(Relation relation,
- List *indexIds);
-
extern void RelationInitIndexAccessInfo(Relation relation);
/* caller must include pg_publication.h */
signature.asc
Description: PGP signature
