Re: Analyze on large changes... - Mailing list pgsql-hackers

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> Seems like a pretty serious (not to say fatal) objection to me.  Surely
> >> you can fix that.
>
> > OK, suggestions.  I know CommandCounterIncrement will not help.  Should
> > I do more pfree'ing?
>
> No, retail pfree'ing is not a maintainable solution.  I was thinking
> more along the lines of a MemoryContextResetAndDeleteChildren() on
> whatever the active context is.  If that doesn't work straight off,
> you might have to create a new working context and switch into it
> before calling the analyze subroutine --- then deleting that context
> would do the trick.

OK, how is this?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/commands/analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/analyze.c,v
retrieving revision 1.35
diff -c -r1.35 analyze.c
*** src/backend/commands/analyze.c    24 May 2002 18:57:55 -0000    1.35
--- src/backend/commands/analyze.c    12 Jun 2002 03:59:45 -0000
***************
*** 156,170 ****
          elevel = DEBUG1;

      /*
-      * Begin a transaction for analyzing this relation.
-      *
-      * Note: All memory allocated during ANALYZE will live in
-      * TransactionCommandContext or a subcontext thereof, so it will all
-      * be released by transaction commit at the end of this routine.
-      */
-     StartTransactionCommand();
-
-     /*
       * Check for user-requested abort.    Note we want this to be inside a
       * transaction, so xact.c doesn't issue useless WARNING.
       */
--- 156,161 ----
***************
*** 177,186 ****
      if (!SearchSysCacheExists(RELOID,
                                ObjectIdGetDatum(relid),
                                0, 0, 0))
-     {
-         CommitTransactionCommand();
          return;
-     }

      /*
       * Open the class, getting only a read lock on it, and check
--- 168,174 ----
***************
*** 196,202 ****
              elog(WARNING, "Skipping \"%s\" --- only table or database owner can ANALYZE it",
                   RelationGetRelationName(onerel));
          relation_close(onerel, AccessShareLock);
-         CommitTransactionCommand();
          return;
      }

--- 184,189 ----
***************
*** 211,217 ****
              elog(WARNING, "Skipping \"%s\" --- can not process indexes, views or special system tables",
                   RelationGetRelationName(onerel));
          relation_close(onerel, AccessShareLock);
-         CommitTransactionCommand();
          return;
      }

--- 198,203 ----
***************
*** 222,228 ****
          strcmp(RelationGetRelationName(onerel), StatisticRelationName) == 0)
      {
          relation_close(onerel, AccessShareLock);
-         CommitTransactionCommand();
          return;
      }

--- 208,213 ----
***************
*** 283,289 ****
      if (attr_cnt <= 0)
      {
          relation_close(onerel, NoLock);
-         CommitTransactionCommand();
          return;
      }

--- 268,273 ----
***************
*** 370,378 ****
       * entries we made in pg_statistic.)
       */
      relation_close(onerel, NoLock);
-
-     /* Commit and release working memory */
-     CommitTransactionCommand();
  }

  /*
--- 354,359 ----
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.226
diff -c -r1.226 vacuum.c
*** src/backend/commands/vacuum.c    24 May 2002 18:57:56 -0000    1.226
--- src/backend/commands/vacuum.c    12 Jun 2002 03:59:54 -0000
***************
*** 110,117 ****


  /* non-export function prototypes */
- static void vacuum_init(VacuumStmt *vacstmt);
- static void vacuum_shutdown(VacuumStmt *vacstmt);
  static List *getrels(const RangeVar *vacrel, const char *stmttype);
  static void vac_update_dbstats(Oid dbid,
                     TransactionId vacuumXID,
--- 110,115 ----
***************
*** 160,165 ****
--- 158,165 ----
  void
  vacuum(VacuumStmt *vacstmt)
  {
+     MemoryContext anl_context,
+                   old_context;
      const char *stmttype = vacstmt->vacuum ? "VACUUM" : "ANALYZE";
      List       *vrl,
                 *cur;
***************
*** 178,190 ****
       * user's transaction too, which would certainly not be the desired
       * behavior.
       */
!     if (IsTransactionBlock())
          elog(ERROR, "%s cannot run inside a BEGIN/END block", stmttype);

      /* Running VACUUM from a function would free the function context */
!     if (!MemoryContextContains(QueryContext, vacstmt))
          elog(ERROR, "%s cannot be executed from a function", stmttype);
!
      /*
       * Send info about dead objects to the statistics collector
       */
--- 178,190 ----
       * user's transaction too, which would certainly not be the desired
       * behavior.
       */
!     if (vacstmt->vacuum && IsTransactionBlock())
          elog(ERROR, "%s cannot run inside a BEGIN/END block", stmttype);

      /* Running VACUUM from a function would free the function context */
!     if (vacstmt->vacuum && !MemoryContextContains(QueryContext, vacstmt))
          elog(ERROR, "%s cannot be executed from a function", stmttype);
!
      /*
       * Send info about dead objects to the statistics collector
       */
***************
*** 203,215 ****
                                          ALLOCSET_DEFAULT_INITSIZE,
                                          ALLOCSET_DEFAULT_MAXSIZE);

      /* Build list of relations to process (note this lives in vac_context) */
      vrl = getrels(vacstmt->relation, stmttype);

      /*
!      * Start up the vacuum cleaner.
       */
!     vacuum_init(vacstmt);

      /*
       * Process each selected relation.    We are careful to process each
--- 203,264 ----
                                          ALLOCSET_DEFAULT_INITSIZE,
                                          ALLOCSET_DEFAULT_MAXSIZE);

+     if (vacstmt->analyze && !vacstmt->vacuum)
+         anl_context = AllocSetContextCreate(QueryContext,
+                                             "Analyze",
+                                             ALLOCSET_DEFAULT_MINSIZE,
+                                             ALLOCSET_DEFAULT_INITSIZE,
+                                             ALLOCSET_DEFAULT_MAXSIZE);
+
      /* Build list of relations to process (note this lives in vac_context) */
      vrl = getrels(vacstmt->relation, stmttype);

      /*
!      *        Formerly, there was code here to prevent more than one VACUUM from
!      *        executing concurrently in the same database.  However, there's no
!      *        good reason to prevent that, and manually removing lockfiles after
!      *        a vacuum crash was a pain for dbadmins.  So, forget about lockfiles,
!      *        and just rely on the locks we grab on each target table
!      *        to ensure that there aren't two VACUUMs running on the same table
!      *        at the same time.
!      *
!      *        The strangeness with committing and starting transactions in the
!      *        init and shutdown routines is due to the fact that the vacuum cleaner
!      *        is invoked via an SQL command, and so is already executing inside
!      *        a transaction.    We need to leave ourselves in a predictable state
!      *        on entry and exit to the vacuum cleaner.  We commit the transaction
!      *        started in PostgresMain() inside vacuum_init(), and start one in
!      *        vacuum_shutdown() to match the commit waiting for us back in
!      *        PostgresMain().
       */
!     if (vacstmt->vacuum)
!     {
!         if (vacstmt->relation == NULL)
!         {
!             /*
!              * Compute the initially applicable OldestXmin and FreezeLimit
!              * XIDs, so that we can record these values at the end of the
!              * VACUUM. Note that individual tables may well be processed with
!              * newer values, but we can guarantee that no (non-shared)
!              * relations are processed with older ones.
!              *
!              * It is okay to record non-shared values in pg_database, even though
!              * we may vacuum shared relations with older cutoffs, because only
!              * the minimum of the values present in pg_database matters.  We
!              * can be sure that shared relations have at some time been
!              * vacuumed with cutoffs no worse than the global minimum; for, if
!              * there is a backend in some other DB with xmin = OLDXMIN that's
!              * determining the cutoff with which we vacuum shared relations,
!              * it is not possible for that database to have a cutoff newer
!              * than OLDXMIN recorded in pg_database.
!              */
!             vacuum_set_xid_limits(vacstmt, false,
!                                   &initialOldestXmin, &initialFreezeLimit);
!         }
!
!         /* matches the StartTransaction in PostgresMain() */
!         CommitTransactionCommand();
!     }

      /*
       * Process each selected relation.    We are careful to process each
***************
*** 225,305 ****
          if (vacstmt->vacuum)
              vacuum_rel(relid, vacstmt, RELKIND_RELATION);
          if (vacstmt->analyze)
              analyze_rel(relid, vacstmt);
      }

      /* clean up */
!     vacuum_shutdown(vacstmt);
! }
!
! /*
!  *    vacuum_init(), vacuum_shutdown() -- start up and shut down the vacuum cleaner.
!  *
!  *        Formerly, there was code here to prevent more than one VACUUM from
!  *        executing concurrently in the same database.  However, there's no
!  *        good reason to prevent that, and manually removing lockfiles after
!  *        a vacuum crash was a pain for dbadmins.  So, forget about lockfiles,
!  *        and just rely on the locks we grab on each target table
!  *        to ensure that there aren't two VACUUMs running on the same table
!  *        at the same time.
!  *
!  *        The strangeness with committing and starting transactions in the
!  *        init and shutdown routines is due to the fact that the vacuum cleaner
!  *        is invoked via an SQL command, and so is already executing inside
!  *        a transaction.    We need to leave ourselves in a predictable state
!  *        on entry and exit to the vacuum cleaner.  We commit the transaction
!  *        started in PostgresMain() inside vacuum_init(), and start one in
!  *        vacuum_shutdown() to match the commit waiting for us back in
!  *        PostgresMain().
!  */
! static void
! vacuum_init(VacuumStmt *vacstmt)
! {
!     if (vacstmt->vacuum && vacstmt->relation == NULL)
      {
!         /*
!          * Compute the initially applicable OldestXmin and FreezeLimit
!          * XIDs, so that we can record these values at the end of the
!          * VACUUM. Note that individual tables may well be processed with
!          * newer values, but we can guarantee that no (non-shared)
!          * relations are processed with older ones.
!          *
!          * It is okay to record non-shared values in pg_database, even though
!          * we may vacuum shared relations with older cutoffs, because only
!          * the minimum of the values present in pg_database matters.  We
!          * can be sure that shared relations have at some time been
!          * vacuumed with cutoffs no worse than the global minimum; for, if
!          * there is a backend in some other DB with xmin = OLDXMIN that's
!          * determining the cutoff with which we vacuum shared relations,
!          * it is not possible for that database to have a cutoff newer
!          * than OLDXMIN recorded in pg_database.
!          */
!         vacuum_set_xid_limits(vacstmt, false,
!                               &initialOldestXmin, &initialFreezeLimit);
!     }
!
!     /* matches the StartTransaction in PostgresMain() */
!     CommitTransactionCommand();
! }

! static void
! vacuum_shutdown(VacuumStmt *vacstmt)
! {
!     /* on entry, we are not in a transaction */
!
!     /* matches the CommitTransaction in PostgresMain() */
!     StartTransactionCommand();

!     /*
!      * If we did a database-wide VACUUM, update the database's pg_database
!      * row with info about the transaction IDs used, and try to truncate
!      * pg_clog.
!      */
!     if (vacstmt->vacuum && vacstmt->relation == NULL)
!     {
!         vac_update_dbstats(MyDatabaseId,
!                            initialOldestXmin, initialFreezeLimit);
!         vac_truncate_clog(initialOldestXmin, initialFreezeLimit);
      }

      /*
--- 274,317 ----
          if (vacstmt->vacuum)
              vacuum_rel(relid, vacstmt, RELKIND_RELATION);
          if (vacstmt->analyze)
+         {
+             /* If we vacuumed, use new transaction for analyze. */
+             if (vacstmt->vacuum)
+                 StartTransactionCommand();
+             else
+                 old_context = MemoryContextSwitchTo(anl_context);
+
              analyze_rel(relid, vacstmt);
+
+             if (vacstmt->vacuum)
+                 CommitTransactionCommand();
+             else
+             {
+                 MemoryContextResetAndDeleteChildren(anl_context);
+                 MemoryContextSwitchTo(old_context);
+             }
+         }
      }

      /* clean up */
!     if (vacstmt->vacuum)
      {
!         /* on entry, we are not in a transaction */

!         /* matches the CommitTransaction in PostgresMain() */
!         StartTransactionCommand();

!         /*
!          * If we did a database-wide VACUUM, update the database's pg_database
!          * row with info about the transaction IDs used, and try to truncate
!          * pg_clog.
!          */
!         if (vacstmt->relation == NULL)
!         {
!             vac_update_dbstats(MyDatabaseId,
!                                initialOldestXmin, initialFreezeLimit);
!             vac_truncate_clog(initialOldestXmin, initialFreezeLimit);
!         }
      }

      /*
***************
*** 309,314 ****
--- 321,330 ----
       */
      MemoryContextDelete(vac_context);
      vac_context = NULL;
+
+     if (vacstmt->analyze && !vacstmt->vacuum)
+         MemoryContextDelete(anl_context);
+
  }

  /*