Andres Freund escribió:

> Patch 02 has changed its shape slightly since the version I posted here,
> because it's a prerequisite for the fix for the multixact bugs around
> heap_freeze_tuple() and heap_tuple_needs_freeze() I've written about
> nearby. I think Alvaro plans to work over my fixes to make them look
> nice enough and commit them soonish.
> Should somebody want to look into it
> http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/multixact-handling
> contains the fixes.

Here's a tweaked version of the freeze patch.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***************
*** 5260,5265 **** heap_inplace_update(Relation relation, HeapTuple tuple)
--- 5260,5267 ----
   * so we need no external state checks to decide what to do.  (This is good
   * because this function is applied during WAL recovery, when we don't have
   * access to any such state, and can't depend on the hint bits to be set.)
+  * There is an exception we make which is to assume GetMultiXactIdMembers can
+  * be called during recovery.
   *
   * Similarly, cutoff_multi must be less than or equal to the smallest
   * MultiXactId used by any transaction currently open.
***************
*** 5281,5288 **** heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
--- 5283,5292 ----
  				  MultiXactId cutoff_multi)
  {
  	bool		changed = false;
+ 	bool		freeze_xmax = false;
  	TransactionId xid;
  
+ 	/* Process xmin */
  	xid = HeapTupleHeaderGetXmin(tuple);
  	if (TransactionIdIsNormal(xid) &&
  		TransactionIdPrecedes(xid, cutoff_xid))
***************
*** 5299,5314 **** heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
  	}
  
  	/*
! 	 * Note that this code handles IS_MULTI Xmax values, too, but only to mark
! 	 * the tuple as not updated if the multixact is below the cutoff Multixact
! 	 * given; it doesn't remove dead members of a very old multixact.
  	 */
  	xid = HeapTupleHeaderGetRawXmax(tuple);
! 	if ((tuple->t_infomask & HEAP_XMAX_IS_MULTI) ?
! 		(MultiXactIdIsValid(xid) &&
! 		 MultiXactIdPrecedes(xid, cutoff_multi)) :
! 		(TransactionIdIsNormal(xid) &&
! 		 TransactionIdPrecedes(xid, cutoff_xid)))
  	{
  		HeapTupleHeaderSetXmax(tuple, InvalidTransactionId);
  
--- 5303,5432 ----
  	}
  
  	/*
! 	 * Process xmax.  To thoroughly examine the current Xmax value we need to
! 	 * resolve a MultiXactId to its member Xids, in case some of them are below
! 	 * the given cutoff for Xids.  In that case, those values might need
! 	 * freezing, too.  Also, if a multi needs freezing, we cannot simply take
! 	 * it out --- if there's a live updater Xid, it needs to be kept.
  	 */
  	xid = HeapTupleHeaderGetRawXmax(tuple);
! 
! 	if (tuple->t_infomask & HEAP_XMAX_INVALID)
! 		;
! 	else if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
! 	{
! 		/* We shouldn't get an invalid multi, but check to avoid bad mojo */
! 		if (!MultiXactIdIsValid(xid))
! 			freeze_xmax = true;
! 		else if (MultiXactIdPrecedes(xid, cutoff_multi))
! 		{
! 			/*
! 			 * An old multi.  If it was a locker only, it can be removed
! 			 * without much further thought; but if it contained an update,
! 			 * we need to examine its status.
! 			 */
! 			if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
! 				freeze_xmax = true;
! 			else
! 			{
! 				TransactionId	update_xid;
! 
! 				/*
! 				 * FIXME this calls TransactionIdDidAbort() internally,
! 				 * falsifying the claim in the comment at the top ...
! 				 */
! 				update_xid = HeapTupleGetUpdateXid(tuple);
! 
! 				/*
! 				 * XXX we rely here on HeapTupleGetUpdateXid returning
! 				 * Invalid for aborted updates.
! 				 */
! 				if (!TransactionIdIsValid(update_xid))
! 					freeze_xmax = true;
! 				else
! 				{
! 					/*
! 					 * A committed or in-progress update.  Careful: if we were
! 					 * to simply set freeze_max to true here, we'd end up with
! 					 * both the old and new tuples being visible.  Handle this
! 					 * by storing the update Xid directly in Xmax and removing
! 					 * the IS_MULTI bit.
! 					 */
! 					Assert(InRecovery || !TransactionIdIsInProgress(cutoff_multi));
! 					tuple->t_infomask &= ~HEAP_XMAX_BITS;
! 					HeapTupleHeaderSetXmax(tuple, update_xid);
! 				}
! 			}
! 		}
! 		else if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
! 		{
! 			/* newer than the cutoff, so don't touch it */
! 			;
! 		}
! 		else
! 		{
! 			MultiXactMember *members;
! 			int			nmembers;
! 			int			i;
! 
! 			/*
! 			 * Need to check whether any members of the multi are below
! 			 * cutoff_xid.  Otherwise, an Xid could survive truncation of its
! 			 * CLOG segment, and we'd get lookup failures when this multi is
! 			 * resolved in the future.
! 			 */
! 			nmembers = GetMultiXactIdMembers(xid, &members, true);
! 			for (i = 0; i < nmembers; i++)
! 			{
! 				Assert(TransactionIdIsNormal(members[i].xid));
! 
! 				/*
! 				 * Ignore pure lockers; those are protected against being read
! 				 * in dangerous situations via OldestVisibleMXactId.
! 				 */
! 				if (!ISUPDATE_from_mxstatus(members[i].status))
! 					continue;
! 
! 				/*
! 				 * Found the update in this multi.  If it's newer than the
! 				 * cutoff for Xids, we needn't do anything; but if it's older,
! 				 * then we need to get rid of it.  Note that the update cannot
! 				 * possibly be a committed transaction (because the tuple
! 				 * should have been removed long ago in that case) or an
! 				 * in-progress transaction (because it couldn't be older than
! 				 * the cutoff_xid).  So it must be an aborted update, and thus
! 				 * it can be removed.
! 				 *
! 				 * FIXME -- what if it's a committed update which has recent
! 				 * new locker transaction?  The tuple wouldn't have been
! 				 * removed in that case (HeapTupleSatisfiesVacuum returns
! 				 * RECENTLY_DEAD).
! 				 */
! 				if (TransactionIdPrecedes(members[i].xid, cutoff_xid))
! 				{
! 					/* only one updater is allowed */
! 					Assert(!freeze_xmax);
! 
! 					/*
! 					 * If below the cutoff_xid, it should have been pruned away
! 					 * earlier.
! 					 */
! 					Assert(InRecovery || TransactionIdDidAbort(members[i].xid));
! 
! 					freeze_xmax = true;
! 				}
! 			}
! 			if (members)
! 				pfree(members);
! 		}
! 	}
! 	else if (TransactionIdIsNormal(xid) &&
! 			 TransactionIdPrecedes(xid, cutoff_xid))
! 	{
! 		freeze_xmax = true;
! 	}
! 
! 	if (freeze_xmax)
  	{
  		HeapTupleHeaderSetXmax(tuple, InvalidTransactionId);
  
***************
*** 5645,5668 **** heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid,
  		TransactionIdPrecedes(xid, cutoff_xid))
  		return true;
  
! 	if (!(tuple->t_infomask & HEAP_XMAX_INVALID))
  	{
! 		if (!(tuple->t_infomask & HEAP_XMAX_IS_MULTI))
! 		{
! 			xid = HeapTupleHeaderGetRawXmax(tuple);
! 			if (TransactionIdIsNormal(xid) &&
! 				TransactionIdPrecedes(xid, cutoff_xid))
! 				return true;
! 		}
  		else
  		{
! 			MultiXactId multi;
  
! 			multi = HeapTupleHeaderGetRawXmax(tuple);
! 			if (MultiXactIdPrecedes(multi, cutoff_multi))
! 				return true;
  		}
  	}
  
  	if (tuple->t_infomask & HEAP_MOVED)
  	{
--- 5763,5813 ----
  		TransactionIdPrecedes(xid, cutoff_xid))
  		return true;
  
! 	if (tuple->t_infomask & HEAP_XMAX_INVALID)
! 		;
! 	else if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
  	{
! 		MultiXactId multi;
! 
! 		multi = HeapTupleHeaderGetRawXmax(tuple);
! 		if (MultiXactIdPrecedes(multi, cutoff_multi))
! 			return true;
! 		else if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
! 			;
  		else
  		{
! 			MultiXactMember *members;
! 			int			nmembers;
! 			int			i;
  
! 			nmembers = GetMultiXactIdMembers(xid, &members, true);
! 			for (i = 0; i < nmembers; i++)
! 			{
! 				TransactionId member = members[i].xid;
! 				Assert(TransactionIdIsNormal(member));
! 
! 				/* we don't care about lockers */
! 				if (members[i].status != MultiXactStatusNoKeyUpdate ||
! 					members[i].status == MultiXactStatusUpdate)
! 					continue;
! 
! 				if (TransactionIdPrecedes(member, cutoff_xid))
! 				{
! 					pfree(members);
! 					return true;
! 				}
! 			}
! 			if (members)
! 				pfree(members);
  		}
  	}
+ 	else
+ 	{
+ 		xid = HeapTupleHeaderGetRawXmax(tuple);
+ 		if (TransactionIdIsNormal(xid) &&
+ 			TransactionIdPrecedes(xid, cutoff_xid))
+ 			return true;
+ 	}
  
  	if (tuple->t_infomask & HEAP_MOVED)
  	{
*** a/src/backend/access/transam/multixact.c
--- b/src/backend/access/transam/multixact.c
***************
*** 444,449 **** MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
--- 444,453 ----
  
  	for (i = 0, j = 0; i < nmembers; i++)
  	{
+ 		/*
+ 		 * FIXME: is it possible that we copy over too old updater xids
+ 		 * here?
+ 		 */
  		if (TransactionIdIsInProgress(members[i].xid) ||
  			((members[i].status > MultiXactStatusForUpdate) &&
  			 TransactionIdDidCommit(members[i].xid)))
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to