Now that we allow ANALYZE to run inside a transaction block, the locks
authorTom Lane
Sun, 11 Aug 2002 00:08:48 +0000 (00:08 +0000)
committerTom Lane
Sun, 11 Aug 2002 00:08:48 +0000 (00:08 +0000)
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'.

src/backend/commands/analyze.c

index 5cabe21d5f927dd6b32288d5650bb3ce28a27280..5dafb1a42adf03e6073f19aa0e3a98332f686b62 100644 (file)
@@ -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,6 +1644,15 @@ 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)
@@ -1651,12 +1660,7 @@ 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);
 }