From 8c19977e9c515cc29af449a7ab6c25e496f539f3 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 19 Apr 2011 18:50:56 -0400 Subject: [PATCH] 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/bootstrap/bootstrap.c | 2 +- src/backend/catalog/heap.c | 2 +- src/backend/catalog/index.c | 38 +++++++++++++++++++++++++------ src/backend/commands/cluster.c | 6 +++++ src/backend/commands/indexcmds.c | 2 +- src/include/catalog/index.h | 3 ++- 6 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 528ea23d4c..fc093ccc4b 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -1134,7 +1134,7 @@ build_indices(void) heap = heap_open(ILHead->il_heap, NoLock); ind = index_open(ILHead->il_ind, NoLock); - index_build(heap, ind, ILHead->il_info, false); + index_build(heap, ind, ILHead->il_info, false, false); index_close(ind, NoLock); heap_close(heap, NoLock); diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 09b26a5c72..8c5670fdb8 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2569,7 +2569,7 @@ RelationTruncateIndexes(Relation heapRelation) /* Initialize the index and rebuild */ /* Note: we do not need to re-establish pkey setting */ - index_build(heapRelation, currentIndex, indexInfo, false); + index_build(heapRelation, currentIndex, indexInfo, false, true); /* We're done with this index */ index_close(currentIndex, NoLock); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 9c96509600..2ccb996112 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1061,7 +1061,7 @@ index_create(Relation heapRelation, } else { - index_build(heapRelation, indexRelation, indexInfo, isprimary); + index_build(heapRelation, indexRelation, indexInfo, isprimary, false); } /* @@ -1680,8 +1680,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. @@ -1691,7 +1694,8 @@ void index_build(Relation heapRelation, Relation indexRelation, IndexInfo *indexInfo, - bool isprimary) + bool isprimary, + bool isreindex) { RegProcedure procedure; IndexBuildResult *stats; @@ -1762,8 +1766,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; @@ -1778,6 +1789,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); @@ -2767,7 +2781,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(); { @@ -2787,6 +2801,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) { diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 10ec32c05d..ff228b7d53 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1398,6 +1398,12 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, * advantage to the other order anyway because this is all transactional, * so no chance to reclaim disk space before commit. We do not need a * final CommandCounterIncrement() because reindex_relation does it. + * + * Note: because index_build is called via reindex_relation, it will never + * set indcheckxmin true for the indexes. This is OK even though in some + * sense we are building new indexes rather than rebuilding existing ones, + * because the new heap won't contain any HOT chains at all, let alone + * broken ones, so it can't be necessary to set indcheckxmin. */ reindex_flags = REINDEX_REL_SUPPRESS_INDEX_USE; if (check_constraints) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2f4e751100..53a6aafbbf 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -505,7 +505,7 @@ DefineIndex(RangeVar *heapRelation, indexInfo->ii_BrokenHotChain = false; /* Now build the index */ - index_build(rel, indexRelation, indexInfo, primary); + index_build(rel, indexRelation, indexInfo, primary, false); /* Close both the relations, but keep the locks */ heap_close(rel, NoLock); diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index cc7e785cc0..071db7f401 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -75,7 +75,8 @@ extern void FormIndexDatum(IndexInfo *indexInfo, extern void index_build(Relation heapRelation, Relation indexRelation, IndexInfo *indexInfo, - bool isprimary); + bool isprimary, + bool isreindex); extern double IndexBuildHeapScan(Relation heapRelation, Relation indexRelation, -- 2.40.0