On 09/08/2010 10:02 AM, Tom Lane wrote:
> Alvaro Herrera <alvhe...@commandprompt.com> writes:
>> Excerpts from Tom Lane's message of mié sep 08 12:12:31 -0400 2010:
>>> AFAIR it doesn't keep the first snapshot around.  If it did, most of
>>> your work on snapshot list trimming would have been useless, no?
> 
>> That's my point precisely.  The name "IsolationUsesXactSnapshot" makes
>> it sound like it applies to any transaction that uses snapshots for
>> isolation, doesn't it?
> 
> I don't think so, at least not when compared to the alternative 
> IsolationUsesStmtSnapshot.

The attached patch is updated for the various comments, as well as some
wording tweaks by me. If there are no objections I'd like to commit this
in a day or two.

Joe

-- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
? src/backend/parser/parse.h
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.293
diff -c -r1.293 heapam.c
*** src/backend/access/heap/heapam.c	29 Jul 2010 16:14:36 -0000	1.293
--- src/backend/access/heap/heapam.c	9 Sep 2010 16:03:18 -0000
***************
*** 2173,2179 ****
  
  	if (crosscheck != InvalidSnapshot && result == HeapTupleMayBeUpdated)
  	{
! 		/* Perform additional check for serializable RI updates */
  		if (!HeapTupleSatisfiesVisibility(&tp, crosscheck, buffer))
  			result = HeapTupleUpdated;
  	}
--- 2173,2179 ----
  
  	if (crosscheck != InvalidSnapshot && result == HeapTupleMayBeUpdated)
  	{
! 		/* Perform additional check for transaction-snapshot mode RI updates */
  		if (!HeapTupleSatisfiesVisibility(&tp, crosscheck, buffer))
  			result = HeapTupleUpdated;
  	}
***************
*** 2525,2531 ****
  
  	if (crosscheck != InvalidSnapshot && result == HeapTupleMayBeUpdated)
  	{
! 		/* Perform additional check for serializable RI updates */
  		if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
  			result = HeapTupleUpdated;
  	}
--- 2525,2531 ----
  
  	if (crosscheck != InvalidSnapshot && result == HeapTupleMayBeUpdated)
  	{
! 		/* Perform additional check for transaction-snapshot mode RI updates */
  		if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
  			result = HeapTupleUpdated;
  	}
Index: src/backend/catalog/index.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.338
diff -c -r1.338 index.c
*** src/backend/catalog/index.c	13 Aug 2010 20:10:50 -0000	1.338
--- src/backend/catalog/index.c	9 Sep 2010 16:03:18 -0000
***************
*** 2049,2055 ****
   *
   * After completing validate_index(), we wait until all transactions that
   * were alive at the time of the reference snapshot are gone; this is
!  * necessary to be sure there are none left with a serializable snapshot
   * older than the reference (and hence possibly able to see tuples we did
   * not index).	Then we mark the index "indisvalid" and commit.  Subsequent
   * transactions will be able to use it for queries.
--- 2049,2055 ----
   *
   * After completing validate_index(), we wait until all transactions that
   * were alive at the time of the reference snapshot are gone; this is
!  * necessary to be sure there are none left with a transaction snapshot
   * older than the reference (and hence possibly able to see tuples we did
   * not index).	Then we mark the index "indisvalid" and commit.  Subsequent
   * transactions will be able to use it for queries.
Index: src/backend/commands/trigger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/trigger.c,v
retrieving revision 1.265
diff -c -r1.265 trigger.c
*** src/backend/commands/trigger.c	19 Aug 2010 15:46:18 -0000	1.265
--- src/backend/commands/trigger.c	9 Sep 2010 16:03:18 -0000
***************
*** 2387,2393 ****
  
  			case HeapTupleUpdated:
  				ReleaseBuffer(buffer);
! 				if (IsXactIsoLevelSerializable)
  					ereport(ERROR,
  							(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  							 errmsg("could not serialize access due to concurrent update")));
--- 2387,2393 ----
  
  			case HeapTupleUpdated:
  				ReleaseBuffer(buffer);
! 				if (IsolationUsesXactSnapshot())
  					ereport(ERROR,
  							(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  							 errmsg("could not serialize access due to concurrent update")));
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.354
diff -c -r1.354 execMain.c
*** src/backend/executor/execMain.c	5 Aug 2010 14:45:02 -0000	1.354
--- src/backend/executor/execMain.c	9 Sep 2010 16:03:19 -0000
***************
*** 1554,1560 ****
  
  				case HeapTupleUpdated:
  					ReleaseBuffer(buffer);
! 					if (IsXactIsoLevelSerializable)
  						ereport(ERROR,
  								(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  								 errmsg("could not serialize access due to concurrent update")));
--- 1554,1560 ----
  
  				case HeapTupleUpdated:
  					ReleaseBuffer(buffer);
! 					if (IsolationUsesXactSnapshot())
  						ereport(ERROR,
  								(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  								 errmsg("could not serialize access due to concurrent update")));
Index: src/backend/executor/nodeLockRows.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/nodeLockRows.c,v
retrieving revision 1.6
diff -c -r1.6 nodeLockRows.c
*** src/backend/executor/nodeLockRows.c	28 Jul 2010 17:21:56 -0000	1.6
--- src/backend/executor/nodeLockRows.c	9 Sep 2010 16:03:19 -0000
***************
*** 130,136 ****
  				break;
  
  			case HeapTupleUpdated:
! 				if (IsXactIsoLevelSerializable)
  					ereport(ERROR,
  							(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  							 errmsg("could not serialize access due to concurrent update")));
--- 130,136 ----
  				break;
  
  			case HeapTupleUpdated:
! 				if (IsolationUsesXactSnapshot())
  					ereport(ERROR,
  							(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  							 errmsg("could not serialize access due to concurrent update")));
Index: src/backend/executor/nodeModifyTable.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/nodeModifyTable.c,v
retrieving revision 1.9
diff -c -r1.9 nodeModifyTable.c
*** src/backend/executor/nodeModifyTable.c	18 Aug 2010 21:52:24 -0000	1.9
--- src/backend/executor/nodeModifyTable.c	9 Sep 2010 16:03:19 -0000
***************
*** 310,316 ****
  	 * Note: if es_crosscheck_snapshot isn't InvalidSnapshot, we check that
  	 * the row to be deleted is visible to that snapshot, and throw a can't-
  	 * serialize error if not.	This is a special-case behavior needed for
! 	 * referential integrity updates in serializable transactions.
  	 */
  ldelete:;
  	result = heap_delete(resultRelationDesc, tupleid,
--- 310,316 ----
  	 * Note: if es_crosscheck_snapshot isn't InvalidSnapshot, we check that
  	 * the row to be deleted is visible to that snapshot, and throw a can't-
  	 * serialize error if not.	This is a special-case behavior needed for
! 	 * referential integrity updates in transaction-snapshot mode transactions.
  	 */
  ldelete:;
  	result = heap_delete(resultRelationDesc, tupleid,
***************
*** 328,334 ****
  			break;
  
  		case HeapTupleUpdated:
! 			if (IsXactIsoLevelSerializable)
  				ereport(ERROR,
  						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  						 errmsg("could not serialize access due to concurrent update")));
--- 328,334 ----
  			break;
  
  		case HeapTupleUpdated:
! 			if (IsolationUsesXactSnapshot())
  				ereport(ERROR,
  						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  						 errmsg("could not serialize access due to concurrent update")));
***************
*** 499,505 ****
  	 * Note: if es_crosscheck_snapshot isn't InvalidSnapshot, we check that
  	 * the row to be updated is visible to that snapshot, and throw a can't-
  	 * serialize error if not.	This is a special-case behavior needed for
! 	 * referential integrity updates in serializable transactions.
  	 */
  	result = heap_update(resultRelationDesc, tupleid, tuple,
  						 &update_ctid, &update_xmax,
--- 499,505 ----
  	 * Note: if es_crosscheck_snapshot isn't InvalidSnapshot, we check that
  	 * the row to be updated is visible to that snapshot, and throw a can't-
  	 * serialize error if not.	This is a special-case behavior needed for
! 	 * referential integrity updates in transaction-snapshot mode transactions.
  	 */
  	result = heap_update(resultRelationDesc, tupleid, tuple,
  						 &update_ctid, &update_xmax,
***************
*** 516,522 ****
  			break;
  
  		case HeapTupleUpdated:
! 			if (IsXactIsoLevelSerializable)
  				ereport(ERROR,
  						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  						 errmsg("could not serialize access due to concurrent update")));
--- 516,522 ----
  			break;
  
  		case HeapTupleUpdated:
! 			if (IsolationUsesXactSnapshot())
  				ereport(ERROR,
  						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  						 errmsg("could not serialize access due to concurrent update")));
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.596
diff -c -r1.596 postgres.c
*** src/backend/tcop/postgres.c	12 Aug 2010 23:24:54 -0000	1.596
--- src/backend/tcop/postgres.c	9 Sep 2010 16:03:19 -0000
***************
*** 2802,2808 ****
  				 *
  				 * PROCSIG_RECOVERY_CONFLICT_SNAPSHOT if no snapshots are held
  				 * by parent transactions and the transaction is not
! 				 * serializable
  				 *
  				 * PROCSIG_RECOVERY_CONFLICT_TABLESPACE if no temp files or
  				 * cursors open in parent transactions
--- 2802,2808 ----
  				 *
  				 * PROCSIG_RECOVERY_CONFLICT_SNAPSHOT if no snapshots are held
  				 * by parent transactions and the transaction is not
! 				 * transaction-snapshot mode
  				 *
  				 * PROCSIG_RECOVERY_CONFLICT_TABLESPACE if no temp files or
  				 * cursors open in parent transactions
Index: src/backend/tcop/pquery.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/pquery.c,v
retrieving revision 1.137
diff -c -r1.137 pquery.c
*** src/backend/tcop/pquery.c	26 Feb 2010 02:01:02 -0000	1.137
--- src/backend/tcop/pquery.c	9 Sep 2010 16:03:19 -0000
***************
*** 1163,1170 ****
  	 * Set snapshot if utility stmt needs one.	Most reliable way to do this
  	 * seems to be to enumerate those that do not need one; this is a short
  	 * list.  Transaction control, LOCK, and SET must *not* set a snapshot
! 	 * since they need to be executable at the start of a serializable
! 	 * transaction without freezing a snapshot.  By extension we allow SHOW
  	 * not to set a snapshot.  The other stmts listed are just efficiency
  	 * hacks.  Beware of listing anything that can modify the database --- if,
  	 * say, it has to update an index with expressions that invoke
--- 1163,1170 ----
  	 * Set snapshot if utility stmt needs one.	Most reliable way to do this
  	 * seems to be to enumerate those that do not need one; this is a short
  	 * list.  Transaction control, LOCK, and SET must *not* set a snapshot
! 	 * since they need to be executable at the start of a transaction-snapshot
! 	 * mode transaction without freezing a snapshot.  By extension we allow SHOW
  	 * not to set a snapshot.  The other stmts listed are just efficiency
  	 * hacks.  Beware of listing anything that can modify the database --- if,
  	 * say, it has to update an index with expressions that invoke
Index: src/backend/utils/adt/ri_triggers.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/ri_triggers.c,v
retrieving revision 1.120
diff -c -r1.120 ri_triggers.c
*** src/backend/utils/adt/ri_triggers.c	28 Jul 2010 05:22:24 -0000	1.120
--- src/backend/utils/adt/ri_triggers.c	9 Sep 2010 16:03:19 -0000
***************
*** 2784,2793 ****
  
  	/*
  	 * Run the plan.  For safety we force a current snapshot to be used. (In
! 	 * serializable mode, this arguably violates serializability, but we
! 	 * really haven't got much choice.)  We don't need to register the
! 	 * snapshot, because SPI_execute_snapshot will see to it.  We need at most
! 	 * one tuple returned, so pass limit = 1.
  	 */
  	spi_result = SPI_execute_snapshot(qplan,
  									  NULL, NULL,
--- 2784,2793 ----
  
  	/*
  	 * Run the plan.  For safety we force a current snapshot to be used. (In
! 	 * transaction-snapshot mode, this arguably violates transaction
! 	 * isolation rules, but we really haven't got much choice.)
! 	 * We don't need to register the snapshot, because SPI_execute_snapshot
! 	 * will see to it. We need at most one tuple returned, so pass limit = 1.
  	 */
  	spi_result = SPI_execute_snapshot(qplan,
  									  NULL, NULL,
***************
*** 3332,3346 ****
  	/*
  	 * In READ COMMITTED mode, we just need to use an up-to-date regular
  	 * snapshot, and we will see all rows that could be interesting. But in
! 	 * SERIALIZABLE mode, we can't change the transaction snapshot. If the
! 	 * caller passes detectNewRows == false then it's okay to do the query
  	 * with the transaction snapshot; otherwise we use a current snapshot, and
  	 * tell the executor to error out if it finds any rows under the current
  	 * snapshot that wouldn't be visible per the transaction snapshot.  Note
  	 * that SPI_execute_snapshot will register the snapshots, so we don't need
  	 * to bother here.
  	 */
! 	if (IsXactIsoLevelSerializable && detectNewRows)
  	{
  		CommandCounterIncrement();		/* be sure all my own work is visible */
  		test_snapshot = GetLatestSnapshot();
--- 3332,3346 ----
  	/*
  	 * In READ COMMITTED mode, we just need to use an up-to-date regular
  	 * snapshot, and we will see all rows that could be interesting. But in
! 	 * transaction-snapshot mode, we can't change the transaction snapshot.
! 	 * If the caller passes detectNewRows == false then it's okay to do the query
  	 * with the transaction snapshot; otherwise we use a current snapshot, and
  	 * tell the executor to error out if it finds any rows under the current
  	 * snapshot that wouldn't be visible per the transaction snapshot.  Note
  	 * that SPI_execute_snapshot will register the snapshots, so we don't need
  	 * to bother here.
  	 */
! 	if (IsolationUsesXactSnapshot() && detectNewRows)
  	{
  		CommandCounterIncrement();		/* be sure all my own work is visible */
  		test_snapshot = GetLatestSnapshot();
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.15
diff -c -r1.15 snapmgr.c
*** src/backend/utils/time/snapmgr.c	26 Feb 2010 02:01:15 -0000	1.15
--- src/backend/utils/time/snapmgr.c	9 Sep 2010 16:03:19 -0000
***************
*** 37,46 ****
  
  
  /*
!  * CurrentSnapshot points to the only snapshot taken in a serializable
!  * transaction, and to the latest one taken in a read-committed transaction.
   * SecondarySnapshot is a snapshot that's always up-to-date as of the current
!  * instant, even on a serializable transaction.  It should only be used for
   * special-purpose code (say, RI checking.)
   *
   * These SnapshotData structs are static to simplify memory allocation
--- 37,46 ----
  
  
  /*
!  * CurrentSnapshot points to the only snapshot taken in transaction-snapshot
!  * mode, and to the latest one taken in a read-committed transaction.
   * SecondarySnapshot is a snapshot that's always up-to-date as of the current
!  * instant, even in transaction-snapshot mode.  It should only be used for
   * special-purpose code (say, RI checking.)
   *
   * These SnapshotData structs are static to simplify memory allocation
***************
*** 97,107 ****
  bool		FirstSnapshotSet = false;
  
  /*
!  * Remembers whether this transaction registered a serializable snapshot at
   * start.  We cannot trust FirstSnapshotSet in combination with
!  * IsXactIsoLevelSerializable, because GUC may be reset before us.
   */
! static bool registered_serializable = false;
  
  
  static Snapshot CopySnapshot(Snapshot snapshot);
--- 97,107 ----
  bool		FirstSnapshotSet = false;
  
  /*
!  * Remembers whether this transaction registered a transaction snapshot at
   * start.  We cannot trust FirstSnapshotSet in combination with
!  * IsolationUsesXactSnapshot(), because GUC may be reset before us.
   */
! static bool registered_xact_snapshot = false;
  
  
  static Snapshot CopySnapshot(Snapshot snapshot);
***************
*** 130,150 ****
  		FirstSnapshotSet = true;
  
  		/*
! 		 * In serializable mode, the first snapshot must live until end of
! 		 * xact regardless of what the caller does with it, so we must
  		 * register it internally here and unregister it at end of xact.
  		 */
! 		if (IsXactIsoLevelSerializable)
  		{
  			CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
  												TopTransactionResourceOwner);
! 			registered_serializable = true;
  		}
  
  		return CurrentSnapshot;
  	}
  
! 	if (IsXactIsoLevelSerializable)
  		return CurrentSnapshot;
  
  	CurrentSnapshot = GetSnapshotData(&CurrentSnapshotData);
--- 130,150 ----
  		FirstSnapshotSet = true;
  
  		/*
! 		 * In transaction-snapshot mode, the first snapshot must live until
! 		 * end of xact regardless of what the caller does with it, so we must
  		 * register it internally here and unregister it at end of xact.
  		 */
! 		if (IsolationUsesXactSnapshot())
  		{
  			CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
  												TopTransactionResourceOwner);
! 			registered_xact_snapshot = true;
  		}
  
  		return CurrentSnapshot;
  	}
  
! 	if (IsolationUsesXactSnapshot())
  		return CurrentSnapshot;
  
  	CurrentSnapshot = GetSnapshotData(&CurrentSnapshotData);
***************
*** 155,161 ****
  /*
   * GetLatestSnapshot
   *		Get a snapshot that is up-to-date as of the current instant,
!  *		even if we are executing in SERIALIZABLE mode.
   */
  Snapshot
  GetLatestSnapshot(void)
--- 155,161 ----
  /*
   * GetLatestSnapshot
   *		Get a snapshot that is up-to-date as of the current instant,
!  *		even if we are executing in transaction-snapshot mode.
   */
  Snapshot
  GetLatestSnapshot(void)
***************
*** 515,527 ****
  AtEarlyCommit_Snapshot(void)
  {
  	/*
! 	 * On a serializable transaction we must unregister our private refcount
! 	 * to the serializable snapshot.
  	 */
! 	if (registered_serializable)
  		UnregisterSnapshotFromOwner(CurrentSnapshot,
  									TopTransactionResourceOwner);
! 	registered_serializable = false;
  
  }
  
--- 515,527 ----
  AtEarlyCommit_Snapshot(void)
  {
  	/*
! 	 * In transaction-snapshot mode we must unregister our private refcount
! 	 * to the transaction-snapshot.
  	 */
! 	if (registered_xact_snapshot)
  		UnregisterSnapshotFromOwner(CurrentSnapshot,
  									TopTransactionResourceOwner);
! 	registered_xact_snapshot = false;
  
  }
  
***************
*** 557,561 ****
  	SecondarySnapshot = NULL;
  
  	FirstSnapshotSet = false;
! 	registered_serializable = false;
  }
--- 557,561 ----
  	SecondarySnapshot = NULL;
  
  	FirstSnapshotSet = false;
! 	registered_xact_snapshot = false;
  }
Index: src/include/access/xact.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/access/xact.h,v
retrieving revision 1.103
diff -c -r1.103 xact.h
*** src/include/access/xact.h	26 Feb 2010 02:01:21 -0000	1.103
--- src/include/access/xact.h	9 Sep 2010 16:03:20 -0000
***************
*** 35,41 ****
   * We only implement two isolation levels internally.  This macro should
   * be used to check which one is selected.
   */
! #define IsXactIsoLevelSerializable (XactIsoLevel >= XACT_REPEATABLE_READ)
  
  /* Xact read-only state */
  extern bool DefaultXactReadOnly;
--- 35,41 ----
   * We only implement two isolation levels internally.  This macro should
   * be used to check which one is selected.
   */
! #define IsolationUsesXactSnapshot() (XactIsoLevel >= XACT_REPEATABLE_READ)
  
  /* Xact read-only state */
  extern bool DefaultXactReadOnly;

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to