v7-0001-Add-skip_locked-argument-to-find_inheritance_chil.patch

application/octet-stream

Filename: v7-0001-Add-skip_locked-argument-to-find_inheritance_chil.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #14941: Vacuum crashes

Patch

Format: format-patch
Series: patch v7-0001
Subject: Add skip_locked argument to find_inheritance_children().
File+
src/backend/catalog/partition.c 1 1
src/backend/catalog/pg_inherits.c 23 3
src/backend/commands/lockcmds.c 1 1
src/backend/commands/tablecmds.c 8 8
src/backend/commands/trigger.c 1 1
src/include/catalog/pg_inherits_fn.h 2 1
From 2e8936a89e9bbc8bd25e611617b435212402bc2b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Thu, 5 Apr 2018 16:45:49 +0000
Subject: [PATCH v7 01/12] Add skip_locked argument to
 find_inheritance_children().

---
 src/backend/catalog/partition.c      |  2 +-
 src/backend/catalog/pg_inherits.c    | 26 +++++++++++++++++++++++---
 src/backend/commands/lockcmds.c      |  2 +-
 src/backend/commands/tablecmds.c     | 16 ++++++++--------
 src/backend/commands/trigger.c       |  2 +-
 src/include/catalog/pg_inherits_fn.h |  3 ++-
 6 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 39ee773d93..42453c5007 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -232,7 +232,7 @@ RelationBuildPartitionDesc(Relation rel)
 	PartitionRangeBound **rbounds = NULL;
 
 	/* Get partition oids from pg_inherits */
-	inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock);
+	inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock, false, NULL);
 
 	/* Collect bound spec nodes in a list */
 	i = 0;
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index 5a5beb9273..17f8b58fba 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -52,9 +52,14 @@ typedef struct SeenRelsEntry
  * given rel; caller should already have locked it).  If lockmode is NoLock
  * then no locks are acquired, but caller must beware of race conditions
  * against possible DROPs of child relations.
+ *
+ * If skip_locked is true, any child relations that cannot be locked immediately
+ * without waiting are not added to the returned OID list, and skipped will be
+ * set to true if it is provided.  Otherwise, skipped will be set to false if it
+ * is provided.
  */
 List *
-find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
+find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, bool skip_locked, bool *skipped)
 {
 	List	   *list = NIL;
 	Relation	relation;
@@ -66,13 +71,19 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
 	int			maxoids,
 				numoids,
 				i;
+	bool		did_skip = false;
 
 	/*
 	 * Can skip the scan if pg_class shows the relation has never had a
 	 * subclass.
 	 */
 	if (!has_subclass(parentrelId))
+	{
+		if (skipped != NULL)
+			*skipped = did_skip;
+
 		return NIL;
+	}
 
 	/*
 	 * Scan pg_inherits and build a working array of subclass OIDs.
@@ -125,7 +136,13 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
 		if (lockmode != NoLock)
 		{
 			/* Get the lock to synchronize against concurrent drop */
-			LockRelationOid(inhrelid, lockmode);
+			if (!skip_locked)
+				LockRelationOid(inhrelid, lockmode);
+			else if (!ConditionalLockRelationOid(inhrelid, lockmode))
+			{
+				did_skip = true;
+				continue;
+			}
 
 			/*
 			 * Now that we have the lock, double-check to see if the relation
@@ -146,6 +163,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
 
 	pfree(oidarr);
 
+	if (skipped != NULL)
+		*skipped = did_skip;
+
 	return list;
 }
 
@@ -200,7 +220,7 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
 		ListCell   *lc;
 
 		/* Get the direct children of this rel */
-		currentchildren = find_inheritance_children(currentrel, lockmode);
+		currentchildren = find_inheritance_children(currentrel, lockmode, false, NULL);
 
 		/*
 		 * Add to the queue only those children not already seen. This avoids
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index b247c0fe2e..1039159946 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -120,7 +120,7 @@ LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid)
 	List	   *children;
 	ListCell   *lc;
 
-	children = find_inheritance_children(reloid, NoLock);
+	children = find_inheritance_children(reloid, NoLock, false, NULL);
 
 	foreach(lc, children)
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ec2f9616ed..15017c5a62 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2694,7 +2694,7 @@ renameatt_internal(Oid myrelid,
 		 * expected_parents will only be 0 if we are not already recursing.
 		 */
 		if (expected_parents == 0 &&
-			find_inheritance_children(myrelid, NoLock) != NIL)
+			find_inheritance_children(myrelid, NoLock, false, NULL) != NIL)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
 					 errmsg("inherited column \"%s\" must be renamed in child tables too",
@@ -2893,7 +2893,7 @@ rename_constraint_internal(Oid myrelid,
 		else
 		{
 			if (expected_parents == 0 &&
-				find_inheritance_children(myrelid, NoLock) != NIL)
+				find_inheritance_children(myrelid, NoLock, false, NULL) != NIL)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
 						 errmsg("inherited constraint \"%s\" must be renamed in child tables too",
@@ -5354,7 +5354,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 */
 	if (colDef->identity &&
 		recurse &&
-		find_inheritance_children(myrelid, NoLock) != NIL)
+		find_inheritance_children(myrelid, NoLock, false, NULL) != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
 				 errmsg("cannot recursively add identity column to table that has child tables")));
@@ -5597,7 +5597,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * routines, we have to do this one level of recursion at a time; we can't
 	 * use find_all_inheritors to do it in one pass.
 	 */
-	children = find_inheritance_children(RelationGetRelid(rel), lockmode);
+	children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL);
 
 	/*
 	 * If we are told not to recurse, there had better not be any child
@@ -6693,7 +6693,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
 	 * routines, we have to do this one level of recursion at a time; we can't
 	 * use find_all_inheritors to do it in one pass.
 	 */
-	children = find_inheritance_children(RelationGetRelid(rel), lockmode);
+	children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL);
 
 	if (children)
 	{
@@ -7144,7 +7144,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * routines, we have to do this one level of recursion at a time; we can't
 	 * use find_all_inheritors to do it in one pass.
 	 */
-	children = find_inheritance_children(RelationGetRelid(rel), lockmode);
+	children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL);
 
 	/*
 	 * Check if ONLY was specified with ALTER TABLE.  If so, allow the
@@ -8831,7 +8831,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	 * use find_all_inheritors to do it in one pass.
 	 */
 	if (!is_no_inherit_constraint)
-		children = find_inheritance_children(RelationGetRelid(rel), lockmode);
+		children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL);
 	else
 		children = NIL;
 
@@ -9173,7 +9173,7 @@ ATPrepAlterColumnType(List **wqueue,
 		}
 	}
 	else if (!recursing &&
-			 find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL)
+			 find_inheritance_children(RelationGetRelid(rel), NoLock, false, NULL) != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
 				 errmsg("type of inherited column \"%s\" must be changed in child tables too",
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index a189356cad..a7e549e0ae 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -1115,7 +1115,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 			ListCell   *l;
 			List	   *idxs = NIL;
 
-			idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock);
+			idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock, false, NULL);
 			foreach(l, idxs)
 				childTbls = lappend_oid(childTbls,
 										IndexGetRelation(lfirst_oid(l),
diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h
index eebee977a5..34d2a2f39f 100644
--- a/src/include/catalog/pg_inherits_fn.h
+++ b/src/include/catalog/pg_inherits_fn.h
@@ -17,7 +17,8 @@
 #include "nodes/pg_list.h"
 #include "storage/lock.h"
 
-extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode);
+extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode,
+						  bool skip_locked, bool *skipped);
 extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode,
 					List **parents);
 extern bool has_subclass(Oid relationId);
-- 
2.16.2