From ebcbc8cfe6c2d9c587ae2a25e545f09b58b666b0 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 19 Apr 2011 18:51:03 -0400 Subject: Avoid changing an index's indcheckxmin horizon during REINDEX. There can never be a need to push the indcheckxmin horizon forward, since any HOT chains that are actually broken with respect to the index must pre-date its original creation. So we can just avoid changing pg_index altogether during a REINDEX operation. This offers a cleaner solution than my previous patch for the problem found a few days ago that we mustn't try to update pg_index while we are reindexing it. System catalog indexes will always be created with indcheckxmin = false during initdb, and with this modified code we should never try to change their pg_index entries. This avoids special-casing system catalogs as the former patch did, and should provide a performance benefit for many cases where REINDEX formerly caused an index to be considered unusable for a short time. Back-patch to 8.3 to cover all versions containing HOT. Note that this patch changes the API for index_build(), but I believe it is unlikely that any add-on code is calling that directly. --- src/backend/catalog/index.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) (limited to 'src/backend/catalog/index.c') diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index caa985a791f..999b5da1fdf 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -957,7 +957,7 @@ index_create(Oid heapRelationId, } else { - index_build(heapRelation, indexRelation, indexInfo, isprimary); + index_build(heapRelation, indexRelation, indexInfo, isprimary, false); } /* @@ -1389,8 +1389,11 @@ index_update_stats(Relation rel, * entries of the index and heap relation as needed, using statistics * returned by ambuild as well as data passed by the caller. * - * Note: when reindexing an existing index, isprimary can be false; - * the index is already properly marked and need not be re-marked. + * isprimary tells whether to mark the index as a primary-key index. + * isreindex indicates we are recreating a previously-existing index. + * + * Note: when reindexing an existing index, isprimary can be false even if + * the index is a PK; it's already properly marked and need not be re-marked. * * Note: before Postgres 8.2, the passed-in heap and index Relations * were automatically closed by this routine. This is no longer the case. @@ -1400,7 +1403,8 @@ void index_build(Relation heapRelation, Relation indexRelation, IndexInfo *indexInfo, - bool isprimary) + bool isprimary, + bool isreindex) { RegProcedure procedure; IndexBuildResult *stats; @@ -1454,8 +1458,15 @@ index_build(Relation heapRelation, * If we found any potentially broken HOT chains, mark the index as not * being usable until the current transaction is below the event horizon. * See src/backend/access/heap/README.HOT for discussion. - */ - if (indexInfo->ii_BrokenHotChain) + * + * However, when reindexing an existing index, we should do nothing here. + * Any HOT chains that are broken with respect to the index must predate + * the index's original creation, so there is no need to change the + * index's usability horizon. Moreover, we *must not* try to change + * the index's pg_index entry while reindexing pg_index itself, and this + * optimization nicely prevents that. + */ + if (indexInfo->ii_BrokenHotChain && !isreindex) { Oid indexId = RelationGetRelid(indexRelation); Relation pg_index; @@ -1470,6 +1481,9 @@ index_build(Relation heapRelation, elog(ERROR, "cache lookup failed for index %u", indexId); indexForm = (Form_pg_index) GETSTRUCT(indexTuple); + /* If it's a new index, indcheckxmin shouldn't be set ... */ + Assert(!indexForm->indcheckxmin); + indexForm->indcheckxmin = true; simple_heap_update(pg_index, &indexTuple->t_self, indexTuple); CatalogUpdateIndexes(pg_index, indexTuple); @@ -2461,7 +2475,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks) /* Initialize the index and rebuild */ /* Note: we do not need to re-establish pkey setting */ - index_build(heapRelation, iRel, indexInfo, false); + index_build(heapRelation, iRel, indexInfo, false, true); } PG_CATCH(); { @@ -2481,6 +2495,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks) * We can also reset indcheckxmin, because we have now done a * non-concurrent index build, *except* in the case where index_build * found some still-broken HOT chains. + * + * Note that it is important to not update the pg_index entry if we don't + * have to, because updating it will move the index's usability horizon + * (recorded as the tuple's xmin value) if indcheckxmin is true. We don't + * really want REINDEX to move the usability horizon forward ever, but we + * have no choice if we are to fix indisvalid or indisready. Of course, + * clearing indcheckxmin eliminates the issue, so we're happy to do that + * if we can. Another reason for caution here is that while reindexing + * pg_index itself, we must not try to update it. We assume that + * pg_index's indexes will always have these flags in their clean state. */ if (!skipped_constraint) { -- cgit v1.2.3