Now that we allow ANALYZE to run inside a transaction block, the locks

tglsfdc · tglsfdc · commit 2e10863bf527 · 2002-08-11T00:08:48.000Z
it takes could be held for quite awhile after the analyze step completes.
Rethink locking of pg_statistic in light of this fact.  The original
scheme took an exclusive lock on pg_statistic, which was okay when the
lock could be expected to be released shortly, but that doesn't hold
anymore.  Back off to a normal writer's lock (RowExclusiveLock).  This
allows concurrent ANALYZE of nonoverlapping sets of tables, at the price
that concurrent ANALYZEs of the same table may fail with 'tuple
concurrently updated'.
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.41 2002/08/05 03:29:16 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.42 2002/08/11 00:08:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -272,7 +272,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
 	 */
 	if (attr_cnt <= 0)
 	{
-		relation_close(onerel, NoLock);
+		relation_close(onerel, AccessShareLock);
 		return;
 	}
 
@@ -1644,19 +1644,23 @@ compare_mcvs(const void *a, const void *b)
  *		This could possibly be made to work, but it's not worth the trouble.
  *		Note analyze_rel() has seen to it that we won't come here when
  *		vacuuming pg_statistic itself.
+ *
+ *		Note: if two backends concurrently try to analyze the same relation,
+ *		the second one is likely to fail here with a "tuple concurrently
+ *		updated" error.  This is slightly annoying, but no real harm is done.
+ *		We could prevent the problem by using a stronger lock on the
+ *		relation for ANALYZE (ie, ShareUpdateExclusiveLock instead
+ *		of AccessShareLock); but that cure seems worse than the disease,
+ *		especially now that ANALYZE doesn't start a new transaction
+ *		for each relation.  The lock could be held for a long time...
  */
 static void
 update_attstats(Oid relid, int natts, VacAttrStats **vacattrstats)
 {
 	Relation	sd;
 	int			attno;
 
-	/*
-	 * We use an ExclusiveLock on pg_statistic to ensure that only one
-	 * backend is writing it at a time --- without that, we might have to
-	 * deal with concurrent updates here, and it's not worth the trouble.
-	 */
-	sd = heap_openr(StatisticRelationName, ExclusiveLock);
+	sd = heap_openr(StatisticRelationName, RowExclusiveLock);
 
 	for (attno = 0; attno < natts; attno++)
 	{
@@ -1789,6 +1793,5 @@ update_attstats(Oid relid, int natts, VacAttrStats **vacattrstats)
 		heap_freetuple(stup);
 	}
 
-	/* close rel, but hold lock till upcoming commit */
-	heap_close(sd, NoLock);
+	heap_close(sd, RowExclusiveLock);
 }

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`*`
`9`	`9`	`*`
`10`	`10`	`* IDENTIFICATION`
`11`		`- * $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.41 2002/08/05 03:29:16 tgl Exp $`
	`11`	`+ * $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.42 2002/08/11 00:08:48 tgl Exp $`
`12`	`12`	`*`
`13`	`13`	`*-------------------------------------------------------------------------`
`14`	`14`	`*/`
`@@ -272,7 +272,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)`
`272`	`272`	`*/`
`273`	`273`	`if (attr_cnt <= 0)`
`274`	`274`	`{`
`275`		`- relation_close(onerel, NoLock);`
	`275`	`+ relation_close(onerel, AccessShareLock);`
`276`	`276`	`return;`
`277`	`277`	`}`
`278`	`278`
`@@ -1644,19 +1644,23 @@ compare_mcvs(const void a, const void b)`
`1644`	`1644`	`* This could possibly be made to work, but it's not worth the trouble.`
`1645`	`1645`	`* Note analyze_rel() has seen to it that we won't come here when`
`1646`	`1646`	`* vacuuming pg_statistic itself.`
	`1647`	`+ *`
	`1648`	`+ * Note: if two backends concurrently try to analyze the same relation,`
	`1649`	`+ * the second one is likely to fail here with a "tuple concurrently`
	`1650`	`+ * updated" error. This is slightly annoying, but no real harm is done.`
	`1651`	`+ * We could prevent the problem by using a stronger lock on the`
	`1652`	`+ * relation for ANALYZE (ie, ShareUpdateExclusiveLock instead`
	`1653`	`+ * of AccessShareLock); but that cure seems worse than the disease,`
	`1654`	`+ * especially now that ANALYZE doesn't start a new transaction`
	`1655`	`+ * for each relation. The lock could be held for a long time...`
`1647`	`1656`	`*/`
`1648`	`1657`	`static void`
`1649`	`1658`	`update_attstats(Oid relid, int natts, VacAttrStats **vacattrstats)`
`1650`	`1659`	`{`
`1651`	`1660`	`Relation sd;`
`1652`	`1661`	`int attno;`
`1653`	`1662`
`1654`		`- /*`
`1655`		`- * We use an ExclusiveLock on pg_statistic to ensure that only one`
`1656`		`- * backend is writing it at a time --- without that, we might have to`
`1657`		`- * deal with concurrent updates here, and it's not worth the trouble.`
`1658`		`- */`
`1659`		`- sd = heap_openr(StatisticRelationName, ExclusiveLock);`
	`1663`	`+ sd = heap_openr(StatisticRelationName, RowExclusiveLock);`
`1660`	`1664`
`1661`	`1665`	`for (attno = 0; attno < natts; attno++)`
`1662`	`1666`	`{`
`@@ -1789,6 +1793,5 @@ update_attstats(Oid relid, int natts, VacAttrStats **vacattrstats)`
`1789`	`1793`	`heap_freetuple(stup);`
`1790`	`1794`	`}`
`1791`	`1795`
`1792`		`- /* close rel, but hold lock till upcoming commit */`
`1793`		`- heap_close(sd, NoLock);`
	`1796`	`+ heap_close(sd, RowExclusiveLock);`
`1794`	`1797`	`}`