pgindent (was Re: Header comments in the recently added files)

*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***************
*** 746,755 **** OldSerXidAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo)
  	Assert(TransactionIdIsValid(tailXid));
  
  	/*
! 	 * If the SLRU is currently unused, zero out the whole active region
! 	 * from tailXid to headXid before taking it into use. Otherwise zero
! 	 * out only any new pages that enter the tailXid-headXid range as we
! 	 * advance headXid.
  	 */
  	if (oldSerXidControl->headPage < 0)
  	{
--- 746,755 ----
  	Assert(TransactionIdIsValid(tailXid));
  
  	/*
! 	 * If the SLRU is currently unused, zero out the whole active region from
! 	 * tailXid to headXid before taking it into use. Otherwise zero out only
! 	 * any new pages that enter the tailXid-headXid range as we advance
! 	 * headXid.
  	 */
  	if (oldSerXidControl->headPage < 0)
  	{
***************
*** 855,862 **** OldSerXidSetActiveSerXmin(TransactionId xid)
  	/*
  	 * When no sxacts are active, nothing overlaps, set the xid values to
  	 * invalid to show that there are no valid entries.  Don't clear headPage,
! 	 * though.  A new xmin might still land on that page, and we don't want
! 	 * to repeatedly zero out the same page.
  	 */
  	if (!TransactionIdIsValid(xid))
  	{
--- 855,862 ----
  	/*
  	 * When no sxacts are active, nothing overlaps, set the xid values to
  	 * invalid to show that there are no valid entries.  Don't clear headPage,
! 	 * though.	A new xmin might still land on that page, and we don't want to
! 	 * repeatedly zero out the same page.
  	 */
  	if (!TransactionIdIsValid(xid))
  	{
***************
*** 901,907 **** OldSerXidSetActiveSerXmin(TransactionId xid)
  void
  CheckPointPredicate(void)
  {
! 	int tailPage;
  
  	LWLockAcquire(OldSerXidLock, LW_EXCLUSIVE);
  
--- 901,907 ----
  void
  CheckPointPredicate(void)
  {
! 	int			tailPage;
  
  	LWLockAcquire(OldSerXidLock, LW_EXCLUSIVE);
  
***************
*** 1317,1328 **** SummarizeOldestCommittedSxact(void)
  	/*
  	 * This function is only called if there are no sxact slots available.
  	 * Some of them must belong to old, already-finished transactions, so
! 	 * there should be something in FinishedSerializableTransactions list
! 	 * that we can summarize. However, there's a race condition: while we
! 	 * were not holding any locks, a transaction might have ended and cleaned
! 	 * up all the finished sxact entries already, freeing up their sxact
! 	 * slots. In that case, we have nothing to do here. The caller will find
! 	 * one of the slots released by the other backend when it retries.
  	 */
  	if (SHMQueueEmpty(FinishedSerializableTransactions))
  	{
--- 1317,1328 ----
  	/*
  	 * This function is only called if there are no sxact slots available.
  	 * Some of them must belong to old, already-finished transactions, so
! 	 * there should be something in FinishedSerializableTransactions list that
! 	 * we can summarize. However, there's a race condition: while we were not
! 	 * holding any locks, a transaction might have ended and cleaned up all
! 	 * the finished sxact entries already, freeing up their sxact slots. In
! 	 * that case, we have nothing to do here. The caller will find one of the
! 	 * slots released by the other backend when it retries.
  	 */
  	if (SHMQueueEmpty(FinishedSerializableTransactions))
  	{
***************
*** 2207,2213 **** PredicateLockTuple(const Relation relation, const HeapTuple tuple)
  	 */
  	if (relation->rd_index == NULL)
  	{
! 		TransactionId	myxid;
  
  		targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
  
--- 2207,2213 ----
  	 */
  	if (relation->rd_index == NULL)
  	{
! 		TransactionId myxid;
  
  		targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
  
***************
*** 2217,2222 **** PredicateLockTuple(const Relation relation, const HeapTuple tuple)
--- 2217,2223 ----
  			if (TransactionIdFollowsOrEquals(targetxmin, TransactionXmin))
  			{
  				TransactionId xid = SubTransGetTopmostTransaction(targetxmin);
+ 
  				if (TransactionIdEquals(xid, myxid))
  				{
  					/* We wrote it; we already have a write lock. */
***************
*** 2266,2272 **** PredicateLockTupleRowVersionLink(const Relation relation,
  	PREDICATELOCKTARGETTAG oldtupletag;
  	PREDICATELOCKTARGETTAG oldpagetag;
  	PREDICATELOCKTARGETTAG newtupletag;
! 	BlockNumber	oldblk,
  				newblk;
  	OffsetNumber oldoff,
  				newoff;
--- 2267,2273 ----
  	PREDICATELOCKTARGETTAG oldtupletag;
  	PREDICATELOCKTARGETTAG oldpagetag;
  	PREDICATELOCKTARGETTAG newtupletag;
! 	BlockNumber oldblk,
  				newblk;
  	OffsetNumber oldoff,
  				newoff;
***************
*** 2302,2311 **** PredicateLockTupleRowVersionLink(const Relation relation,
  
  	/*
  	 * A page-level lock on the page containing the old tuple counts too.
! 	 * Anyone holding a lock on the page is logically holding a lock on
! 	 * the old tuple, so we need to acquire a lock on his behalf on the
! 	 * new tuple too. However, if the new tuple is on the same page as the
! 	 * old one, the old page-level lock already covers the new tuple.
  	 *
  	 * A relation-level lock always covers both tuple versions, so we don't
  	 * need to worry about those here.
--- 2303,2312 ----
  
  	/*
  	 * A page-level lock on the page containing the old tuple counts too.
! 	 * Anyone holding a lock on the page is logically holding a lock on the
! 	 * old tuple, so we need to acquire a lock on his behalf on the new tuple
! 	 * too. However, if the new tuple is on the same page as the old one, the
! 	 * old page-level lock already covers the new tuple.
  	 *
  	 * A relation-level lock always covers both tuple versions, so we don't
  	 * need to worry about those here.
***************
*** 2662,2671 **** PredicateLockPageSplit(const Relation relation, const BlockNumber oldblkno,
  		/*
  		 * Move the locks to the parent. This shouldn't fail.
  		 *
! 		 * Note that here we are removing locks held by other
! 		 * backends, leading to a possible inconsistency in their
! 		 * local lock hash table. This is OK because we're replacing
! 		 * it with a lock that covers the old one.
  		 */
  		success = TransferPredicateLocksToNewTarget(oldtargettag,
  													newtargettag,
--- 2663,2672 ----
  		/*
  		 * Move the locks to the parent. This shouldn't fail.
  		 *
! 		 * Note that here we are removing locks held by other backends,
! 		 * leading to a possible inconsistency in their local lock hash table.
! 		 * This is OK because we're replacing it with a lock that covers the
! 		 * old one.
  		 */
  		success = TransferPredicateLocksToNewTarget(oldtargettag,
  													newtargettag,
***************
*** 2690,2705 **** PredicateLockPageCombine(const Relation relation, const BlockNumber oldblkno,
  						 const BlockNumber newblkno)
  {
  	/*
! 	 * Page combines differ from page splits in that we ought to be
! 	 * able to remove the locks on the old page after transferring
! 	 * them to the new page, instead of duplicating them. However,
! 	 * because we can't edit other backends' local lock tables,
! 	 * removing the old lock would leave them with an entry in their
! 	 * LocalPredicateLockHash for a lock they're not holding, which
! 	 * isn't acceptable. So we wind up having to do the same work as a
! 	 * page split, acquiring a lock on the new page and keeping the old
! 	 * page locked too. That can lead to some false positives, but
! 	 * should be rare in practice.
  	 */
  	PredicateLockPageSplit(relation, oldblkno, newblkno);
  }
--- 2691,2705 ----
  						 const BlockNumber newblkno)
  {
  	/*
! 	 * Page combines differ from page splits in that we ought to be able to
! 	 * remove the locks on the old page after transferring them to the new
! 	 * page, instead of duplicating them. However, because we can't edit other
! 	 * backends' local lock tables, removing the old lock would leave them
! 	 * with an entry in their LocalPredicateLockHash for a lock they're not
! 	 * holding, which isn't acceptable. So we wind up having to do the same
! 	 * work as a page split, acquiring a lock on the new page and keeping the
! 	 * old page locked too. That can lead to some false positives, but should
! 	 * be rare in practice.
  	 */
  	PredicateLockPageSplit(relation, oldblkno, newblkno);
  }
***************
*** 3710,3717 **** CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
  					/*
  					 * Remove entry in local lock table if it exists and has
  					 * no children. It's OK if it doesn't exist; that means
! 					 * the lock was transferred to a new target by a
! 					 * different backend.
  					 */
  					if (locallock != NULL)
  					{
--- 3710,3717 ----
  					/*
  					 * Remove entry in local lock table if it exists and has
  					 * no children. It's OK if it doesn't exist; that means
! 					 * the lock was transferred to a new target by a different
! 					 * backend.
  					 */
  					if (locallock != NULL)
  					{
***************
*** 3721,3728 **** CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
  						{
  							rmlocallock = (LOCALPREDICATELOCK *)
  								hash_search_with_hash_value(LocalPredicateLockHash,
! 															targettag, targettaghash,
! 															HASH_REMOVE, NULL);
  							Assert(rmlocallock == locallock);
  						}
  					}
--- 3721,3728 ----
  						{
  							rmlocallock = (LOCALPREDICATELOCK *)
  								hash_search_with_hash_value(LocalPredicateLockHash,
! 													targettag, targettaghash,
! 														  HASH_REMOVE, NULL);
  							Assert(rmlocallock == locallock);
  						}
  					}
***************
*** 3827,3834 **** CheckForSerializableConflictIn(const Relation relation, const HeapTuple tuple,
  										 relation->rd_node.dbNode,
  										 relation->rd_id,
  						 ItemPointerGetBlockNumber(&(tuple->t_data->t_ctid)),
! 					   ItemPointerGetOffsetNumber(&(tuple->t_data->t_ctid)),
! 					   HeapTupleHeaderGetXmin(tuple->t_data));
  		CheckTargetForConflictsIn(&targettag);
  	}
  
--- 3827,3834 ----
  										 relation->rd_node.dbNode,
  										 relation->rd_id,
  						 ItemPointerGetBlockNumber(&(tuple->t_data->t_ctid)),
! 						ItemPointerGetOffsetNumber(&(tuple->t_data->t_ctid)),
! 									  HeapTupleHeaderGetXmin(tuple->t_data));
  		CheckTargetForConflictsIn(&targettag);
  	}
  
*** a/src/include/storage/predicate_internals.h
--- b/src/include/storage/predicate_internals.h
***************
*** 266,272 **** typedef struct PREDICATELOCKTARGETTAG
   * version, before the reading transaction is obsolete, we need some way to
   * prevent errors from reuse of a tuple ID.  Rather than attempting to clean
   * up the targets as the related tuples are pruned or vacuumed, we check the
!  * xmin on access.  This should be far less costly.
   */
  typedef struct PREDICATELOCKTARGET PREDICATELOCKTARGET;
  
--- 266,272 ----
   * version, before the reading transaction is obsolete, we need some way to
   * prevent errors from reuse of a tuple ID.  Rather than attempting to clean
   * up the targets as the related tuples are pruned or vacuumed, we check the
!  * xmin on access.	This should be far less costly.
   */
  typedef struct PREDICATELOCKTARGET PREDICATELOCKTARGET;