From: Peter Geoghegan <pg@bowt.ie>
Date: Fri, 10 Aug 2018 20:01:33 +0000 (-0700)
Subject: Handle parallel index builds on mapped relations.
X-Git-Tag: REL_11_BETA4~124
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9353d94a9b70c6cbe181f78e49b2e8c1dc38eada;p=postgresql

Handle parallel index builds on mapped relations.

Commit 9da0cc35284, which introduced parallel CREATE INDEX, failed to
propagate relmapper.c backend local cache state to parallel worker
processes.  This could result in parallel index builds against mapped
catalog relations where the leader process (participating as a worker)
scans the new, pristine relfilenode, while worker processes scan the
obsolescent relfilenode.  When this happened, the final index structure
was typically not consistent with the owning table's structure.  The
final index structure could contain entries formed from both heap
relfilenodes.  Only rebuilds on mapped catalog relations that occur as
part of a VACUUM FULL or CLUSTER could become corrupt in practice, since
their mapped relation relfilenode swap is what allows the inconsistency
to arise.

On master, fix the problem by propagating the required relmapper.c
backend state as part of standard parallel initialization (Cf. commit
29d58fd3).  On v11, simply disallow builds against mapped catalog
relations by deeming them parallel unsafe.

Author: Peter Geoghegan
Reported-By: "death lock"
Reviewed-By: Tom Lane, Amit Kapila
Bug: #15309
Discussion: https://postgr.es/m/153329671686.1405.18298309097348420351@wrigleys.postgresql.org
Backpatch: 11-, where parallel CREATE INDEX was introduced.
---

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 00db38e90b..173f0d21fe 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -6093,11 +6093,13 @@ plan_create_index_workers(Oid tableOid, Oid indexOid)
 	/*
 	 * Determine if it's safe to proceed.
 	 *
-	 * Currently, parallel workers can't access the leader's temporary tables.
-	 * Furthermore, any index predicate or index expressions must be parallel
-	 * safe.
+	 * Currently, parallel workers can't access the leader's temporary tables,
+	 * or the leader's relmapper.c state, which is needed for builds on mapped
+	 * relations.  Furthermore, any index predicate or index expressions must
+	 * be parallel safe.
 	 */
 	if (heap->rd_rel->relpersistence == RELPERSISTENCE_TEMP ||
+		RelationIsMapped(heap) ||
 		!is_parallel_safe(root, (Node *) RelationGetIndexExpressions(index)) ||
 		!is_parallel_safe(root, (Node *) RelationGetIndexPredicate(index)))
 	{
diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c
index 99d095f2df..e3a5e4e386 100644
--- a/src/backend/utils/cache/relmapper.c
+++ b/src/backend/utils/cache/relmapper.c
@@ -263,13 +263,17 @@ RelationMapUpdateMap(Oid relationId, Oid fileNode, bool shared,
 	else
 	{
 		/*
-		 * We don't currently support map changes within subtransactions. This
-		 * could be done with more bookkeeping infrastructure, but it doesn't
-		 * presently seem worth it.
+		 * We don't currently support map changes within subtransactions, and
+		 * parallel workers must avoid relying on mapping state, since it
+		 * isn't propagated from the leader.  This could be done with more
+		 * bookkeeping infrastructure, but it doesn't presently seem worth it.
 		 */
 		if (GetCurrentTransactionNestLevel() > 1)
 			elog(ERROR, "cannot change relation mapping within subtransaction");
 
+		if (IsInParallelMode())
+			elog(ERROR, "cannot change relation mapping in parallel mode");
+
 		if (immediate)
 		{
 			/* Make it active, but only locally */