v7-0011-Skip-VACUUM-FULL-with-VACOPT_SKIP_LOCKED-if-clust.patch

application/octet-stream

Filename: v7-0011-Skip-VACUUM-FULL-with-VACOPT_SKIP_LOCKED-if-clust.patch
Type: application/octet-stream
Part: 10
Message: Re: BUG #14941: Vacuum crashes

Patch

Format: format-patch
Series: patch v7-0011
Subject: Skip VACUUM FULL with VACOPT_SKIP_LOCKED if cluster_rel() would block.
File+
src/backend/commands/cluster.c 25 12
src/backend/commands/vacuum.c 3 2
src/include/commands/cluster.h 2 2
From 81f01e3cc649bd752944524dddc9382b90d8d0a5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Thu, 5 Apr 2018 19:04:25 +0000
Subject: [PATCH v7 11/12] Skip VACUUM FULL with VACOPT_SKIP_LOCKED if
 cluster_rel() would block.

---
 src/backend/commands/cluster.c | 37 +++++++++++++++++++++++++------------
 src/backend/commands/vacuum.c  |  5 +++--
 src/include/commands/cluster.h |  4 ++--
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index d088dc11a6..63486bf34f 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -186,7 +186,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
 		heap_close(rel, NoLock);
 
 		/* Do the job. */
-		cluster_rel(tableOid, indexOid, false, stmt->verbose);
+		cluster_rel(tableOid, indexOid, false, stmt->verbose, false);
 	}
 	else
 	{
@@ -234,7 +234,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
 			/* functions in indexes may want a snapshot set */
 			PushActiveSnapshot(GetTransactionSnapshot());
 			/* Do the job. */
-			cluster_rel(rvtc->tableOid, rvtc->indexOid, true, stmt->verbose);
+			cluster_rel(rvtc->tableOid, rvtc->indexOid, true, stmt->verbose, false);
 			PopActiveSnapshot();
 			CommitTransactionCommand();
 		}
@@ -263,9 +263,15 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
  * If indexOid is InvalidOid, the table will be rewritten in physical order
  * instead of index order.  This is the new implementation of VACUUM FULL,
  * and error messages should refer to the operation as VACUUM not CLUSTER.
+ *
+ * If skip_locked is true and the relation cannot be immediately locked,
+ * processing will be skipped and false will be returned (otherwise, true is
+ * returned).  Note that skip_locked will not prevent blocking on the index
+ * provided.  Since this functionality is only used for VACUUM FULL, this
+ * omission has no effect.
  */
-void
-cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
+bool
+cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose, bool skip_locked)
 {
 	Relation	OldHeap;
 
@@ -278,11 +284,16 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 	 * case, since cluster() already did it.)  The index lock is taken inside
 	 * check_index_is_clusterable.
 	 */
-	OldHeap = try_relation_open(tableOid, AccessExclusiveLock);
+	if (!skip_locked)
+		OldHeap = try_relation_open(tableOid, AccessExclusiveLock);
+	else if (ConditionalLockRelationOid(tableOid, AccessExclusiveLock))
+		OldHeap = try_relation_open(tableOid, NoLock);
+	else
+		return false;
 
 	/* If the table has gone away, we can skip processing it */
 	if (!OldHeap)
-		return;
+		return true;
 
 	/*
 	 * Since we may open a new transaction for each relation, we have to check
@@ -301,7 +312,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 		if (!pg_class_ownercheck(tableOid, GetUserId()))
 		{
 			relation_close(OldHeap, AccessExclusiveLock);
-			return;
+			return true;
 		}
 
 		/*
@@ -315,7 +326,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 		if (RELATION_IS_OTHER_TEMP(OldHeap))
 		{
 			relation_close(OldHeap, AccessExclusiveLock);
-			return;
+			return true;
 		}
 
 		if (OidIsValid(indexOid))
@@ -326,7 +337,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid)))
 			{
 				relation_close(OldHeap, AccessExclusiveLock);
-				return;
+				return true;
 			}
 
 			/*
@@ -336,14 +347,14 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 			if (!HeapTupleIsValid(tuple))	/* probably can't happen */
 			{
 				relation_close(OldHeap, AccessExclusiveLock);
-				return;
+				return true;
 			}
 			indexForm = (Form_pg_index) GETSTRUCT(tuple);
 			if (!indexForm->indisclustered)
 			{
 				ReleaseSysCache(tuple);
 				relation_close(OldHeap, AccessExclusiveLock);
-				return;
+				return true;
 			}
 			ReleaseSysCache(tuple);
 		}
@@ -397,7 +408,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 		!RelationIsPopulated(OldHeap))
 	{
 		relation_close(OldHeap, AccessExclusiveLock);
-		return;
+		return true;
 	}
 
 	/*
@@ -412,6 +423,8 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
 	rebuild_relation(OldHeap, indexOid, verbose);
 
 	/* NB: rebuild_relation does heap_close() on OldHeap */
+
+	return true;
 }
 
 /*
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index f19d112af6..74349f30d9 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1558,8 +1558,9 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params)
 		onerel = NULL;
 
 		/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
-		cluster_rel(relid, InvalidOid, false,
-					(options & VACOPT_VERBOSE) != 0);
+		skipped = !cluster_rel(relid, InvalidOid, false,
+							   (options & VACOPT_VERBOSE) != 0,
+							   (options & VACOPT_SKIP_LOCKED) != 0);
 	}
 	else
 		skipped = !lazy_vacuum_rel(onerel, options, params, vac_strategy);
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index b338cb1098..49bb35fafe 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -19,8 +19,8 @@
 
 
 extern void cluster(ClusterStmt *stmt, bool isTopLevel);
-extern void cluster_rel(Oid tableOid, Oid indexOid, bool recheck,
-			bool verbose);
+extern bool cluster_rel(Oid tableOid, Oid indexOid, bool recheck,
+			bool verbose, bool skip_locked);
 extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
 						   bool recheck, LOCKMODE lockmode);
 extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
-- 
2.16.2