From 8c19977e9c515cc29af449a7ab6c25e496f539f3 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
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 528ea23d4c3..fc093ccc4b9 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 09b26a5c724..8c5670fdb8c 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 9c96509600d..2ccb9961122 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 10ec32c05de..ff228b7d531 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 2f4e751100d..53a6aafbbfb 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 cc7e785cc0a..071db7f4012 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,
--
GitLab