On Fri, Jun 3, 2016 at 4:24 PM, Kevin Grittner <kgri...@gmail.com> wrote:
> Consequently, causing the index to be ignored in planning when
> using the old index
That last line should have read "using an old snapshot"
> is not a nice optimization, but necessary for
> correctness. We already have logic to do this for other cases
> (like HOT updates), so it is a matter of tying in to that existing
> code correctly. This won't be all that novel.
Just to demonstrate that, the minimal patch to fix behavior in this
area would be:
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 31a1438..6c379da 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2268,6 +2268,9 @@ IndexBuildHeapRangeScan(Relation heapRelation,
Assert(numblocks == InvalidBlockNumber);
}
+ if (old_snapshot_threshold >= 0)
+ indexInfo->ii_BrokenHotChain = true;
+
reltuples = 0;
/*
Of course, ii_BrokenHotChain should be renamed to something like
ii_UnsafeForOldSnapshots, and some comments need to be updated; but
the above is the substance of it.
Attached is what I have so far. I'm still looking at what other
comments might need to be adjusted, but thought I should put this
much out for comment now.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 31a1438..b072985 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1672,7 +1672,7 @@ BuildIndexInfo(Relation index)
/* initialize index-build state to default */
ii->ii_Concurrent = false;
- ii->ii_BrokenHotChain = false;
+ ii->ii_UnsafeForOldSnapshots = false;
return ii;
}
@@ -2060,7 +2060,7 @@ index_build(Relation heapRelation,
* about any concurrent readers of the tuple; no other transaction can see
* it yet.
*/
- if (indexInfo->ii_BrokenHotChain && !isreindex &&
+ if (indexInfo->ii_UnsafeForOldSnapshots && !isreindex &&
!indexInfo->ii_Concurrent)
{
Oid indexId = RelationGetRelid(indexRelation);
@@ -2137,11 +2137,11 @@ index_build(Relation heapRelation,
* so here because the AM might reject some of the tuples for its own reasons,
* such as being unable to store NULLs.
*
- * A side effect is to set indexInfo->ii_BrokenHotChain to true if we detect
- * any potentially broken HOT chains. Currently, we set this if there are
- * any RECENTLY_DEAD or DELETE_IN_PROGRESS entries in a HOT chain, without
- * trying very hard to detect whether they're really incompatible with the
- * chain tip.
+ * A side effect is to set indexInfo->ii_UnsafeForOldSnapshots to true if we
+ * detect any potentially broken HOT chains or if old_snapshot_threshold is
+ * set to allow early pruning/vacuuuming. Currently, we set this based on
+ * fairly simple tests; it is not guaranteed that using the index would cause
+ * a problem when set, but if the flag is clear it is guaranteed to be safe.
*/
double
IndexBuildHeapScan(Relation heapRelation,
@@ -2268,6 +2268,14 @@ IndexBuildHeapRangeScan(Relation heapRelation,
Assert(numblocks == InvalidBlockNumber);
}
+ /*
+ * If allowing early pruning/vacuuming, the index cannot be considered
+ * safe for old snapshots, since entries could be missing and lsn is set
+ * to InvalidXLogRecPtr in all pages of the new index.
+ */
+ if (old_snapshot_threshold >= 0)
+ indexInfo->ii_UnsafeForOldSnapshots = true;
+
reltuples = 0;
/*
@@ -2361,7 +2369,7 @@ IndexBuildHeapRangeScan(Relation heapRelation,
{
indexIt = false;
/* mark the index as unsafe for old snapshots */
- indexInfo->ii_BrokenHotChain = true;
+ indexInfo->ii_UnsafeForOldSnapshots = true;
}
else
indexIt = true;
@@ -2488,7 +2496,7 @@ IndexBuildHeapRangeScan(Relation heapRelation,
*/
indexIt = false;
/* mark the index as unsafe for old snapshots */
- indexInfo->ii_BrokenHotChain = true;
+ indexInfo->ii_UnsafeForOldSnapshots = true;
}
else
{
@@ -3409,9 +3417,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
!indexForm->indisready ||
!indexForm->indislive);
if (index_bad ||
- (indexForm->indcheckxmin && !indexInfo->ii_BrokenHotChain))
+ (indexForm->indcheckxmin && !indexInfo->ii_UnsafeForOldSnapshots))
{
- if (!indexInfo->ii_BrokenHotChain)
+ if (!indexInfo->ii_UnsafeForOldSnapshots)
indexForm->indcheckxmin = false;
else if (index_bad)
indexForm->indcheckxmin = true;
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 564e10e..8dcac7e 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -314,7 +314,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
indexInfo->ii_Unique = true;
indexInfo->ii_ReadyForInserts = true;
indexInfo->ii_Concurrent = false;
- indexInfo->ii_BrokenHotChain = false;
+ indexInfo->ii_UnsafeForOldSnapshots = false;
collationObjectId[0] = InvalidOid;
collationObjectId[1] = InvalidOid;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index d14d540..a3d8bd2 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -557,7 +557,7 @@ DefineIndex(Oid relationId,
/* In a concurrent build, mark it not-ready-for-inserts */
indexInfo->ii_ReadyForInserts = !stmt->concurrent;
indexInfo->ii_Concurrent = stmt->concurrent;
- indexInfo->ii_BrokenHotChain = false;
+ indexInfo->ii_UnsafeForOldSnapshots = false;
typeObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
collationObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
@@ -761,7 +761,7 @@ DefineIndex(Oid relationId,
indexInfo = BuildIndexInfo(indexRelation);
Assert(!indexInfo->ii_ReadyForInserts);
indexInfo->ii_Concurrent = true;
- indexInfo->ii_BrokenHotChain = false;
+ indexInfo->ii_UnsafeForOldSnapshots = false;
/* Now build the index */
index_build(rel, indexRelation, indexInfo, stmt->primary, false);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index ee4e189..2ba4230 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -49,10 +49,12 @@
* Unique is it a unique index?
* ReadyForInserts is it valid for inserts?
* Concurrent are we doing a concurrent index build?
- * BrokenHotChain did we detect any broken HOT chains?
+ * UnsafeForOldSnapshots did we detect reasons not to use the index with
+ * old snapshots (like broken HOT chains or enabled
+ * early pruning/vacuuming)?
*
- * ii_Concurrent and ii_BrokenHotChain are used only during index build;
- * they're conventionally set to false otherwise.
+ * ii_Concurrent and ii_UnsafeForOldSnapshots are used only during index
+ * build; they're conventionally set to false otherwise.
* ----------------
*/
typedef struct IndexInfo
@@ -73,7 +75,7 @@ typedef struct IndexInfo
bool ii_Unique;
bool ii_ReadyForInserts;
bool ii_Concurrent;
- bool ii_BrokenHotChain;
+ bool ii_UnsafeForOldSnapshots;
} IndexInfo;
/* ----------------
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
