Remove SnapshotNow and HeapTupleSatisfiesNow.
authorRobert Haas
Thu, 1 Aug 2013 14:46:19 +0000 (10:46 -0400)
committerRobert Haas
Thu, 1 Aug 2013 14:46:19 +0000 (10:46 -0400)
We now use MVCC catalog scans, and, per discussion, have eliminated
all other remaining uses of SnapshotNow, so that we can now get rid of
it.  This will break third-party code which is still using it, which
is intentional, as we want such code to be updated to do things the
new way.

src/backend/catalog/index.c
src/backend/catalog/pg_enum.c
src/backend/commands/cluster.c
src/backend/utils/time/tqual.c
src/include/utils/tqual.h

index 7e4d7c0578c668f4177008ecbadf1e2bf30597e5..b73ee4f2d19f877b30806ce5a092caa4b8b4ed88 100644 (file)
@@ -1258,10 +1258,8 @@ index_constraint_create(Relation heapRelation,
    /*
     * If needed, mark the index as primary and/or deferred in pg_index.
     *
-    * Note: since this is a transactional update, it's unsafe against
-    * concurrent SnapshotNow scans of pg_index.  When making an existing
-    * index into a constraint, caller must have a table lock that prevents
-    * concurrent table updates; if it's less than a full exclusive lock,
+    * Note: When making an existing index into a constraint, caller must
+    * have a table lock that prevents concurrent table updates; otherwise,
     * there is a risk that concurrent readers of the table will miss seeing
     * this index at all.
     */
@@ -2989,17 +2987,18 @@ validate_index_heapscan(Relation heapRelation,
  * index_set_state_flags - adjust pg_index state flags
  *
  * This is used during CREATE/DROP INDEX CONCURRENTLY to adjust the pg_index
- * flags that denote the index's state.  We must use an in-place update of
- * the pg_index tuple, because we do not have exclusive lock on the parent
- * table and so other sessions might concurrently be doing SnapshotNow scans
- * of pg_index to identify the table's indexes.  A transactional update would
- * risk somebody not seeing the index at all.  Because the update is not
+ * flags that denote the index's state.  Because the update is not
  * transactional and will not roll back on error, this must only be used as
  * the last step in a transaction that has not made any transactional catalog
  * updates!
  *
  * Note that heap_inplace_update does send a cache inval message for the
  * tuple, so other sessions will hear about the update as soon as we commit.
+ *
+ * NB: In releases prior to PostgreSQL 9.4, the use of a non-transactional
+ * update here would have been unsafe; now that MVCC rules apply even for
+ * system catalog scans, we could potentially use a transactional update here
+ * instead.
  */
 void
 index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
@@ -3207,14 +3206,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
     * be conservative.)  In this case advancing the usability horizon is
     * appropriate.
     *
-    * Note that if we have to update the tuple, there is a risk of concurrent
-    * transactions not seeing it during their SnapshotNow scans of pg_index.
-    * While not especially desirable, this is safe because no such
-    * transaction could be trying to update the table (since we have
-    * ShareLock on it).  The worst case is that someone might transiently
-    * fail to use the index for a query --- but it was probably unusable
-    * before anyway, if we are updating the tuple.
-    *
     * Another reason for avoiding unnecessary updates here is that while
     * reindexing pg_index itself, we must not try to update tuples in it.
     * pg_index's indexes should always have these flags in their clean state,
index a7ef8cda59656b950934ba4f2dd2f2da99f72def..88711e985e9fc20bd309a260f629a837111c4444 100644 (file)
@@ -462,30 +462,9 @@ restart:
  *     Renumber existing enum elements to have sort positions 1..n.
  *
  * We avoid doing this unless absolutely necessary; in most installations
- * it will never happen.  The reason is that updating existing pg_enum
- * entries creates hazards for other backends that are concurrently reading
- * pg_enum with SnapshotNow semantics. A concurrent SnapshotNow scan could
- * see both old and new versions of an updated row as valid, or neither of
- * them, if the commit happens between scanning the two versions.  It's
- * also quite likely for a concurrent scan to see an inconsistent set of
- * rows (some members updated, some not).
- *
- * We can avoid these risks by reading pg_enum with an MVCC snapshot
- * instead of SnapshotNow, but that forecloses use of the syscaches.
- * We therefore make the following choices:
- *
- * 1. Any code that is interested in the enumsortorder values MUST read
- * pg_enum with an MVCC snapshot, or else acquire lock on the enum type
- * to prevent concurrent execution of AddEnumLabel().  The risk of
- * seeing inconsistent values of enumsortorder is too high otherwise.
- *
- * 2. Code that is not examining enumsortorder can use a syscache
- * (for example, enum_in and enum_out do so).  The worst that can happen
- * is a transient failure to find any valid value of the row.  This is
- * judged acceptable in view of the infrequency of use of RenumberEnumType.
- *
- * XXX. Now that we have MVCC catalog scans, the above reasoning is no longer
- * correct.  Should we revisit any decisions here?
+ * it will never happen.  Before we had MVCC catalog scans, this function
+ * posed various concurrency hazards.  It no longer does, but it's still
+ * extra work, so we don't do it unless it's needed.
  */
 static void
 RenumberEnumType(Relation pg_enum, HeapTuple *existing, int nelems)
index 4519c00e22363fa32b6097543c5fb03af7042621..051b806aa72d8111b17e640d16f88f8f96b92583 100644 (file)
@@ -476,16 +476,6 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
  * mark_index_clustered: mark the specified index as the one clustered on
  *
  * With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
- *
- * Note: we do transactional updates of the pg_index rows, which are unsafe
- * against concurrent SnapshotNow scans of pg_index.  Therefore this is unsafe
- * to execute with less than full exclusive lock on the parent table;
- * otherwise concurrent executions of RelationGetIndexList could miss indexes.
- *
- * XXX: Now that we have MVCC catalog access, SnapshotNow scans of pg_index
- * shouldn't be common enough to worry about.  The above comment needs
- * to be updated, and it may be possible to simplify the logic here in other
- * ways also.
  */
 void
 mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
index da2ce1825e043cc5bd21a48f31bdd2438df659b5..38bb704a4d8ae4a2a7ab21ac5dfbf749de3e3b5e 100644 (file)
  *
  *  HeapTupleSatisfiesMVCC()
  *       visible to supplied snapshot, excludes current command
- *  HeapTupleSatisfiesNow()
- *       visible to instant snapshot, excludes current command
  *  HeapTupleSatisfiesUpdate()
- *       like HeapTupleSatisfiesNow(), but with user-supplied command
+ *       visible to instant snapshot, with user-supplied command
  *       counter and more complex result
  *  HeapTupleSatisfiesSelf()
  *       visible to instant snapshot and current command
@@ -68,7 +66,6 @@
 
 
 /* Static variables representing various special snapshot semantics */
-SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow};
 SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
 SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
 SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
@@ -323,212 +320,6 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
    return false;
 }
 
-/*
- * HeapTupleSatisfiesNow
- *     True iff heap tuple is valid "now".
- *
- * Here, we consider the effects of:
- *     all committed transactions (as of the current instant)
- *     previous commands of this transaction
- *
- * Note we do _not_ include changes made by the current command.  This
- * solves the "Halloween problem" wherein an UPDATE might try to re-update
- * its own output tuples, http://en.wikipedia.org/wiki/Halloween_Problem.
- *
- * Note:
- *     Assumes heap tuple is valid.
- *
- * The satisfaction of "now" requires the following:
- *
- * ((Xmin == my-transaction &&             inserted by the current transaction
- *  Cmin < my-command &&                   before this command, and
- *  (Xmax is null ||                       the row has not been deleted, or
- *   (Xmax == my-transaction &&            it was deleted by the current transaction
- *    Cmax >= my-command)))                but not before this command,
- * ||                                      or
- * (Xmin is committed &&                   the row was inserted by a committed transaction, and
- *     (Xmax is null ||                    the row has not been deleted, or
- *      (Xmax == my-transaction &&         the row is being deleted by this transaction
- *       Cmax >= my-command) ||            but it's not deleted "yet", or
- *      (Xmax != my-transaction &&         the row was deleted by another transaction
- *       Xmax is not committed))))         that has not been committed
- *
- */
-bool
-HeapTupleSatisfiesNow(HeapTuple htup, Snapshot snapshot, Buffer buffer)
-{
-   HeapTupleHeader tuple = htup->t_data;
-   Assert(ItemPointerIsValid(&htup->t_self));
-   Assert(htup->t_tableOid != InvalidOid);
-
-   if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
-   {
-       if (tuple->t_infomask & HEAP_XMIN_INVALID)
-           return false;
-
-       /* Used by pre-9.0 binary upgrades */
-       if (tuple->t_infomask & HEAP_MOVED_OFF)
-       {
-           TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
-
-           if (TransactionIdIsCurrentTransactionId(xvac))
-               return false;
-           if (!TransactionIdIsInProgress(xvac))
-           {
-               if (TransactionIdDidCommit(xvac))
-               {
-                   SetHintBits(tuple, buffer, HEAP_XMIN_INVALID,
-                               InvalidTransactionId);
-                   return false;
-               }
-               SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
-                           InvalidTransactionId);
-           }
-       }
-       /* Used by pre-9.0 binary upgrades */
-       else if (tuple->t_infomask & HEAP_MOVED_IN)
-       {
-           TransactionId xvac = HeapTupleHeaderGetXvac(tuple);
-
-           if (!TransactionIdIsCurrentTransactionId(xvac))
-           {
-               if (TransactionIdIsInProgress(xvac))
-                   return false;
-               if (TransactionIdDidCommit(xvac))
-                   SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
-                               InvalidTransactionId);
-               else
-               {
-                   SetHintBits(tuple, buffer, HEAP_XMIN_INVALID,
-                               InvalidTransactionId);
-                   return false;
-               }
-           }
-       }
-       else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
-       {
-           if (HeapTupleHeaderGetCmin(tuple) >= GetCurrentCommandId(false))
-               return false;   /* inserted after scan started */
-
-           if (tuple->t_infomask & HEAP_XMAX_INVALID)  /* xid invalid */
-               return true;
-
-           if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))    /* not deleter */
-               return true;
-
-           if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
-           {
-               TransactionId xmax;
-
-               xmax = HeapTupleGetUpdateXid(tuple);
-               if (!TransactionIdIsValid(xmax))
-                   return true;
-
-               /* updating subtransaction must have aborted */
-               if (!TransactionIdIsCurrentTransactionId(xmax))
-                   return true;
-               else
-                   return false;
-           }
-
-           if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))
-           {
-               /* deleting subtransaction must have aborted */
-               SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
-                           InvalidTransactionId);
-               return true;
-           }
-
-           if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false))
-               return true;    /* deleted after scan started */
-           else
-               return false;   /* deleted before scan started */
-       }
-       else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
-           return false;
-       else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
-           SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
-                       HeapTupleHeaderGetXmin(tuple));
-       else
-       {
-           /* it must have aborted or crashed */
-           SetHintBits(tuple, buffer, HEAP_XMIN_INVALID,
-                       InvalidTransactionId);
-           return false;
-       }
-   }
-
-   /* by here, the inserting transaction has committed */
-
-   if (tuple->t_infomask & HEAP_XMAX_INVALID)  /* xid invalid or aborted */
-       return true;
-
-   if (tuple->t_infomask & HEAP_XMAX_COMMITTED)
-   {
-       if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
-           return true;
-       return false;
-   }
-
-   if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
-   {
-       TransactionId xmax;
-
-       if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
-           return true;
-
-       xmax = HeapTupleGetUpdateXid(tuple);
-       if (!TransactionIdIsValid(xmax))
-           return true;
-       if (TransactionIdIsCurrentTransactionId(xmax))
-       {
-           if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false))
-               return true;    /* deleted after scan started */
-           else
-               return false;   /* deleted before scan started */
-       }
-       if (TransactionIdIsInProgress(xmax))
-           return true;
-       if (TransactionIdDidCommit(xmax))
-           return false;
-       return true;
-   }
-
-   if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))
-   {
-       if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
-           return true;
-       if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false))
-           return true;        /* deleted after scan started */
-       else
-           return false;       /* deleted before scan started */
-   }
-
-   if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmax(tuple)))
-       return true;
-
-   if (!TransactionIdDidCommit(HeapTupleHeaderGetRawXmax(tuple)))
-   {
-       /* it must have aborted or crashed */
-       SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
-                   InvalidTransactionId);
-       return true;
-   }
-
-   /* xmax transaction committed */
-
-   if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
-   {
-       SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
-                   InvalidTransactionId);
-       return true;
-   }
-
-   SetHintBits(tuple, buffer, HEAP_XMAX_COMMITTED,
-               HeapTupleHeaderGetRawXmax(tuple));
-   return false;
-}
-
 /*
  * HeapTupleSatisfiesAny
  *     Dummy "satisfies" routine: any tuple satisfies SnapshotAny.
@@ -614,10 +405,10 @@ HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot,
 /*
  * HeapTupleSatisfiesUpdate
  *
- * Same logic as HeapTupleSatisfiesNow, but returns a more detailed result
- * code, since UPDATE needs to know more than "is it visible?".  Also,
- * tuples of my own xact are tested against the passed CommandId not
- * CurrentCommandId.
+ * This function returns a more detailed result code than most of the
+ * functions in this file, since UPDATE needs to know more than "is it
+ * visible?".  It also allows for user-supplied CommandId rather than
+ * relying on CurrentCommandId.
  *
  * The possible return codes are:
  *
@@ -1051,10 +842,6 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
  *     transactions started after the snapshot was taken
  *     changes made by the current command
  *
- * This is the same as HeapTupleSatisfiesNow, except that transactions that
- * were in progress or as yet unstarted when the snapshot was taken will
- * be treated as uncommitted, even if they have committed by now.
- *
  * (Notice, however, that the tuple status hint bits will be updated on the
  * basis of the true state of the transaction, even if we then pretend we
  * can't see it.)
index 800e366f30b51d1fde19d53548fb09c0b0fb7a25..19f56e4b4d34feb189a462e98f99a925fcdaa052 100644 (file)
 
 
 /* Static variables representing various special snapshot semantics */
-extern PGDLLIMPORT SnapshotData SnapshotNowData;
 extern PGDLLIMPORT SnapshotData SnapshotSelfData;
 extern PGDLLIMPORT SnapshotData SnapshotAnyData;
 extern PGDLLIMPORT SnapshotData SnapshotToastData;
 
-#define SnapshotNow            (&SnapshotNowData)
 #define SnapshotSelf       (&SnapshotSelfData)
 #define SnapshotAny            (&SnapshotAnyData)
 #define SnapshotToast      (&SnapshotToastData)
@@ -67,8 +65,6 @@ typedef enum
 /* These are the "satisfies" test routines for the various snapshot types */
 extern bool HeapTupleSatisfiesMVCC(HeapTuple htup,
                       Snapshot snapshot, Buffer buffer);
-extern bool HeapTupleSatisfiesNow(HeapTuple htup,
-                     Snapshot snapshot, Buffer buffer);
 extern bool HeapTupleSatisfiesSelf(HeapTuple htup,
                       Snapshot snapshot, Buffer buffer);
 extern bool HeapTupleSatisfiesAny(HeapTuple htup,