Improvement of procArray.xmin for VACUUM - Mailing list pgsql-patches

VACUUM treats dead tuple with an xmax less than all procArray.xmin's as
invisible with space ready to be reused.

Right now, we set procArray.xmin at transaction start to the required
SERIALIZABLE value.  We currently don't update procArray.xmin when we
know we are in a READ COMMITTED transaction, even though we already call
GetSnapshotData().

This means that if a transaction completes that was active when our
transaction started, we don't update our procArray.xmin during the next
multi-statement transaction command to indicate that we don't care about
the completed transaction anymore.

I have been thinking we could improve how quickly VACUUM can expire rows
if we update procArray.xmin more frequently for non-SERIALIZABLE
transactions.

The attached patch updates procArray.xmin in this manner.  Here is an
example of how the patch improves dead row reuse:

Session #:
        1                               2               3

        CREATE TABLE test(x int);
        INSERT INTO test VALUES (1);
        BEGIN;
        DELETE FROM test;
                                        BEGIN;
                                        SELECT 1;
        COMMIT;
                                                        VACUUM VERBOSE test;
                                                        (row not reused)
                                        SELECT 1;

                                        (At this point #2 doesn't see
                                         the test row anymore.  Patch
                                         updates procArray.xmin.)

                                                        VACUUM VERBOSE test;
                                                        (row reused with patch)
                                        COMMIT;
                                                        VACUUM VERBOSE test;
                                                        (normal row reuse)

What the patch does is to recompute procArray.xmin during the second
SELECT, which is then used by the next VACUUM.

The patch has debug code that prints the procArray.xmin assignments.  I
am not sure if I have all the boolean calls to GetTransactionSnapshot()
correct.  The set_procArray_xmin parameter should be true only when we
are sure we aren't going to be resetting the session back to an earlier
snapshot.

The major affect of the patch is to set the minimum procArray.xmin to be
the earliest in-process transaction, rather than the earliest in-process
transaction at transaction start.

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/catalog/index.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.280
diff -c -c -r1.280 index.c
*** src/backend/catalog/index.c    3 Mar 2007 20:08:41 -0000    1.280
--- src/backend/catalog/index.c    23 Mar 2007 19:20:44 -0000
***************
*** 1394,1400 ****
      }
      else if (indexInfo->ii_Concurrent)
      {
!         snapshot = CopySnapshot(GetTransactionSnapshot());
          OldestXmin = InvalidTransactionId;        /* not used */
      }
      else
--- 1394,1400 ----
      }
      else if (indexInfo->ii_Concurrent)
      {
!         snapshot = CopySnapshot(GetTransactionSnapshot(false));
          OldestXmin = InvalidTransactionId;        /* not used */
      }
      else
Index: src/backend/commands/cluster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/cluster.c,v
retrieving revision 1.157
diff -c -c -r1.157 cluster.c
*** src/backend/commands/cluster.c    13 Mar 2007 00:33:39 -0000    1.157
--- src/backend/commands/cluster.c    23 Mar 2007 19:20:45 -0000
***************
*** 204,210 ****
              /* Start a new transaction for each relation. */
              StartTransactionCommand();
              /* functions in indexes may want a snapshot set */
!             ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
              cluster_rel(rvtc, true);
              CommitTransactionCommand();
          }
--- 204,210 ----
              /* Start a new transaction for each relation. */
              StartTransactionCommand();
              /* functions in indexes may want a snapshot set */
!             ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(true));
              cluster_rel(rvtc, true);
              CommitTransactionCommand();
          }
Index: src/backend/commands/indexcmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v
retrieving revision 1.157
diff -c -c -r1.157 indexcmds.c
*** src/backend/commands/indexcmds.c    13 Mar 2007 00:33:39 -0000    1.157
--- src/backend/commands/indexcmds.c    23 Mar 2007 19:20:45 -0000
***************
*** 493,499 ****
       * We also set ActiveSnapshot to this snap, since functions in indexes may
       * need a snapshot.
       */
!     snapshot = CopySnapshot(GetTransactionSnapshot());
      ActiveSnapshot = snapshot;

      /*
--- 493,499 ----
       * We also set ActiveSnapshot to this snap, since functions in indexes may
       * need a snapshot.
       */
!     snapshot = CopySnapshot(GetTransactionSnapshot(false));
      ActiveSnapshot = snapshot;

      /*
***************
*** 1304,1310 ****

          StartTransactionCommand();
          /* functions in indexes may want a snapshot set */
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
          if (reindex_relation(relid, true))
              ereport(NOTICE,
                      (errmsg("table \"%s\" was reindexed",
--- 1304,1310 ----

          StartTransactionCommand();
          /* functions in indexes may want a snapshot set */
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(true));
          if (reindex_relation(relid, true))
              ereport(NOTICE,
                      (errmsg("table \"%s\" was reindexed",
Index: src/backend/commands/trigger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/trigger.c,v
retrieving revision 1.214
diff -c -c -r1.214 trigger.c
*** src/backend/commands/trigger.c    19 Mar 2007 23:38:29 -0000    1.214
--- src/backend/commands/trigger.c    23 Mar 2007 19:20:46 -0000
***************
*** 2645,2651 ****
       */
      events = &afterTriggers->events;
      if (events->head != NULL)
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());

      /*
       * Run all the remaining triggers.    Loop until they are all gone, just in
--- 2645,2651 ----
       */
      events = &afterTriggers->events;
      if (events->head != NULL)
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));

      /*
       * Run all the remaining triggers.    Loop until they are all gone, just in
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.349
diff -c -c -r1.349 vacuum.c
*** src/backend/commands/vacuum.c    14 Mar 2007 18:48:55 -0000    1.349
--- src/backend/commands/vacuum.c    23 Mar 2007 19:20:47 -0000
***************
*** 412,418 ****
                  {
                      StartTransactionCommand();
                      /* functions in indexes may want a snapshot set */
!                     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
                  }
                  else
                      old_context = MemoryContextSwitchTo(anl_context);
--- 412,418 ----
                  {
                      StartTransactionCommand();
                      /* functions in indexes may want a snapshot set */
!                     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));
                  }
                  else
                      old_context = MemoryContextSwitchTo(anl_context);
***************
*** 470,476 ****
           * necessary if we are called from autovacuum because autovacuum might
           * continue on to do an ANALYZE-only call.
           */
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
      }

      if (vacstmt->vacuum && !IsAutoVacuumWorkerProcess())
--- 470,476 ----
           * necessary if we are called from autovacuum because autovacuum might
           * continue on to do an ANALYZE-only call.
           */
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));
      }

      if (vacstmt->vacuum && !IsAutoVacuumWorkerProcess())
***************
*** 964,970 ****
      if (vacstmt->full)
      {
          /* functions in indexes may want a snapshot set */
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
      }
      else
      {
--- 964,970 ----
      if (vacstmt->full)
      {
          /* functions in indexes may want a snapshot set */
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));
      }
      else
      {
Index: src/backend/executor/functions.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/functions.c,v
retrieving revision 1.112
diff -c -c -r1.112 functions.c
*** src/backend/executor/functions.c    13 Mar 2007 00:33:40 -0000    1.112
--- src/backend/executor/functions.c    23 Mar 2007 19:20:48 -0000
***************
*** 300,306 ****
      else
      {
          CommandCounterIncrement();
!         snapshot = CopySnapshot(GetTransactionSnapshot());
      }

      if (IsA(es->stmt, PlannedStmt))
--- 300,306 ----
      else
      {
          CommandCounterIncrement();
!         snapshot = CopySnapshot(GetTransactionSnapshot(false));
      }

      if (IsA(es->stmt, PlannedStmt))
Index: src/backend/executor/spi.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/spi.c,v
retrieving revision 1.173
diff -c -c -r1.173 spi.c
*** src/backend/executor/spi.c    17 Mar 2007 03:15:38 -0000    1.173
--- src/backend/executor/spi.c    23 Mar 2007 19:20:48 -0000
***************
*** 997,1003 ****
      else
      {
          CommandCounterIncrement();
!         snapshot = GetTransactionSnapshot();
      }

      /*
--- 997,1003 ----
      else
      {
          CommandCounterIncrement();
!         snapshot = GetTransactionSnapshot(false);
      }

      /*
***************
*** 1530,1536 ****
                      if (read_only)
                          ActiveSnapshot = CopySnapshot(saveActiveSnapshot);
                      else
!                         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
                  }
                  else
                  {
--- 1530,1536 ----
                      if (read_only)
                          ActiveSnapshot = CopySnapshot(saveActiveSnapshot);
                      else
!                         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));
                  }
                  else
                  {
Index: src/backend/postmaster/autovacuum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.34
diff -c -c -r1.34 autovacuum.c
*** src/backend/postmaster/autovacuum.c    13 Mar 2007 00:33:41 -0000    1.34
--- src/backend/postmaster/autovacuum.c    23 Mar 2007 19:20:49 -0000
***************
*** 868,874 ****
      StartTransactionCommand();

      /* functions in indexes may want a snapshot set */
!     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());

      /*
       * Clean up any dead statistics collector entries for this DB. We always
--- 868,874 ----
      StartTransactionCommand();

      /* functions in indexes may want a snapshot set */
!     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));

      /*
       * Clean up any dead statistics collector entries for this DB. We always
Index: src/backend/storage/ipc/procarray.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/storage/ipc/procarray.c,v
retrieving revision 1.22
diff -c -c -r1.22 procarray.c
*** src/backend/storage/ipc/procarray.c    23 Mar 2007 03:16:39 -0000    1.22
--- src/backend/storage/ipc/procarray.c    23 Mar 2007 19:20:49 -0000
***************
*** 498,504 ****
   *----------
   */
  Snapshot
! GetSnapshotData(Snapshot snapshot, bool serializable)
  {
      ProcArrayStruct *arrayP = procArray;
      TransactionId xmin;
--- 498,504 ----
   *----------
   */
  Snapshot
! GetSnapshotData(Snapshot snapshot, bool set_procArray_xmin)
  {
      ProcArrayStruct *arrayP = procArray;
      TransactionId xmin;
***************
*** 510,519 ****

      Assert(snapshot != NULL);

!     /* Serializable snapshot must be computed before any other... */
!     Assert(serializable ?
!            !TransactionIdIsValid(MyProc->xmin) :
!            TransactionIdIsValid(MyProc->xmin));

      /*
       * Allocating space for maxProcs xids is usually overkill; numProcs would
--- 510,517 ----

      Assert(snapshot != NULL);

!     /* Either we set an xmin, or we already have one.  */
!     Assert(set_procArray_xmin || TransactionIdIsValid(MyProc->xmin));

      /*
       * Allocating space for maxProcs xids is usually overkill; numProcs would
***************
*** 656,664 ****
          }
      }

!     if (serializable)
          MyProc->xmin = TransactionXmin = xmin;
!
      LWLockRelease(ProcArrayLock);

      /*
--- 654,668 ----
          }
      }

!     if (set_procArray_xmin)
!     {
!         if (xmin != MyProc->xmin)
!             fprintf(stderr, "%x:%x: set new xmin old=%x new=%x\n", MyProcPid, MyProc->xid, MyProc->xmin, xmin);
!         else
!             fprintf(stderr, "%x:%x: not set\n", MyProcPid, MyProc->xid);
          MyProc->xmin = TransactionXmin = xmin;
!     }
!
      LWLockRelease(ProcArrayLock);

      /*
Index: src/backend/tcop/fastpath.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v
retrieving revision 1.96
diff -c -c -r1.96 fastpath.c
*** src/backend/tcop/fastpath.c    3 Mar 2007 19:32:54 -0000    1.96
--- src/backend/tcop/fastpath.c    23 Mar 2007 19:20:49 -0000
***************
*** 308,314 ****
       * Now that we know we are in a valid transaction, set snapshot in case
       * needed by function itself or one of the datatype I/O routines.
       */
!     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());

      /*
       * Begin parsing the buffer contents.
--- 308,314 ----
       * Now that we know we are in a valid transaction, set snapshot in case
       * needed by function itself or one of the datatype I/O routines.
       */
!     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));

      /*
       * Begin parsing the buffer contents.
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.529
diff -c -c -r1.529 postgres.c
*** src/backend/tcop/postgres.c    22 Mar 2007 19:55:04 -0000    1.529
--- src/backend/tcop/postgres.c    23 Mar 2007 19:20:51 -0000
***************
*** 738,744 ****
          {
              if (needSnapshot)
              {
!                 ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
                  needSnapshot = false;
              }
              stmt = (Node *) pg_plan_query(query, boundParams);
--- 738,744 ----
          {
              if (needSnapshot)
              {
!                 ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(true));
                  needSnapshot = false;
              }
              stmt = (Node *) pg_plan_query(query, boundParams);
Index: src/backend/tcop/pquery.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/pquery.c,v
retrieving revision 1.115
diff -c -c -r1.115 pquery.c
*** src/backend/tcop/pquery.c    13 Mar 2007 00:33:42 -0000    1.115
--- src/backend/tcop/pquery.c    23 Mar 2007 19:20:51 -0000
***************
*** 154,160 ****
       * Must always set snapshot for plannable queries.    Note we assume that
       * caller will take care of restoring ActiveSnapshot on exit/error.
       */
!     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());

      /*
       * Create the QueryDesc object
--- 154,160 ----
       * Must always set snapshot for plannable queries.    Note we assume that
       * caller will take care of restoring ActiveSnapshot on exit/error.
       */
!     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));

      /*
       * Create the QueryDesc object
***************
*** 489,495 ****
                  if (snapshot)
                      ActiveSnapshot = CopySnapshot(snapshot);
                  else
!                     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());

                  /*
                   * Create QueryDesc in portal's context; for the moment, set
--- 489,495 ----
                  if (snapshot)
                      ActiveSnapshot = CopySnapshot(snapshot);
                  else
!                     ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));

                  /*
                   * Create QueryDesc in portal's context; for the moment, set
***************
*** 1163,1169 ****
            IsA(utilityStmt, NotifyStmt) ||
            IsA(utilityStmt, UnlistenStmt) ||
            IsA(utilityStmt, CheckPointStmt)))
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
      else
          ActiveSnapshot = NULL;

--- 1163,1169 ----
            IsA(utilityStmt, NotifyStmt) ||
            IsA(utilityStmt, UnlistenStmt) ||
            IsA(utilityStmt, CheckPointStmt)))
!         ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));
      else
          ActiveSnapshot = NULL;

Index: src/backend/utils/adt/ri_triggers.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/ri_triggers.c,v
retrieving revision 1.92
diff -c -c -r1.92 ri_triggers.c
*** src/backend/utils/adt/ri_triggers.c    15 Mar 2007 23:12:06 -0000    1.92
--- src/backend/utils/adt/ri_triggers.c    23 Mar 2007 19:20:52 -0000
***************
*** 3283,3289 ****
      {
          CommandCounterIncrement();        /* be sure all my own work is visible */
          test_snapshot = CopySnapshot(GetLatestSnapshot());
!         crosscheck_snapshot = CopySnapshot(GetTransactionSnapshot());
      }
      else
      {
--- 3283,3289 ----
      {
          CommandCounterIncrement();        /* be sure all my own work is visible */
          test_snapshot = CopySnapshot(GetLatestSnapshot());
!         crosscheck_snapshot = CopySnapshot(GetTransactionSnapshot(false));
      }
      else
      {
Index: src/backend/utils/time/tqual.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/time/tqual.c,v
retrieving revision 1.101
diff -c -c -r1.101 tqual.c
*** src/backend/utils/time/tqual.c    5 Jan 2007 22:19:47 -0000    1.101
--- src/backend/utils/time/tqual.c    23 Mar 2007 19:20:53 -0000
***************
*** 1202,1208 ****
   * the result with CopySnapshot() if it is to be used very long.
   */
  Snapshot
! GetTransactionSnapshot(void)
  {
      /* First call in transaction? */
      if (SerializableSnapshot == NULL)
--- 1202,1208 ----
   * the result with CopySnapshot() if it is to be used very long.
   */
  Snapshot
! GetTransactionSnapshot(bool set_procArray_xmin)
  {
      /* First call in transaction? */
      if (SerializableSnapshot == NULL)
***************
*** 1214,1220 ****
      if (IsXactIsoLevelSerializable)
          return SerializableSnapshot;

!     LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);

      return LatestSnapshot;
  }
--- 1214,1221 ----
      if (IsXactIsoLevelSerializable)
          return SerializableSnapshot;

!     LatestSnapshot = GetSnapshotData(&LatestSnapshotData,
!     /* we know we are not Serializable */ set_procArray_xmin);

      return LatestSnapshot;
  }
Index: src/include/utils/tqual.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/tqual.h,v
retrieving revision 1.65
diff -c -c -r1.65 tqual.h
*** src/include/utils/tqual.h    5 Jan 2007 22:19:59 -0000    1.65
--- src/include/utils/tqual.h    23 Mar 2007 19:20:54 -0000
***************
*** 142,154 ****
  extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTupleHeader tuple,
                           TransactionId OldestXmin, Buffer buffer);

! extern Snapshot GetTransactionSnapshot(void);
  extern Snapshot GetLatestSnapshot(void);
  extern Snapshot CopySnapshot(Snapshot snapshot);
  extern void FreeSnapshot(Snapshot snapshot);
  extern void FreeXactSnapshot(void);

  /* in procarray.c; declared here to avoid including tqual.h in procarray.h: */
! extern Snapshot GetSnapshotData(Snapshot snapshot, bool serializable);

  #endif   /* TQUAL_H */
--- 142,154 ----
  extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTupleHeader tuple,
                           TransactionId OldestXmin, Buffer buffer);

! extern Snapshot GetTransactionSnapshot(bool set_procArray_xmin);
  extern Snapshot GetLatestSnapshot(void);
  extern Snapshot CopySnapshot(Snapshot snapshot);
  extern void FreeSnapshot(Snapshot snapshot);
  extern void FreeXactSnapshot(void);

  /* in procarray.c; declared here to avoid including tqual.h in procarray.h: */
! extern Snapshot GetSnapshotData(Snapshot snapshot, bool set_procArray_xmin);

  #endif   /* TQUAL_H */
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.190
diff -c -c -r1.190 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c    15 Mar 2007 23:12:07 -0000    1.190
--- src/pl/plpgsql/src/pl_exec.c    23 Mar 2007 19:20:55 -0000
***************
*** 4124,4130 ****
          if (!estate->readonly_func)
          {
              CommandCounterIncrement();
!             ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
          }

          /*
--- 4124,4130 ----
          if (!estate->readonly_func)
          {
              CommandCounterIncrement();
!             ActiveSnapshot = CopySnapshot(GetTransactionSnapshot(false));
          }

          /*