Re: PATCH to allow concurrent VACUUMs to not lock each

Hannu Krosing wrote:
> *** src/backend/access/transam/xact.c	28 Apr 2005 21:47:10 -0000	1.200
> --- src/backend/access/transam/xact.c	17 May 2005 22:06:34 -0000
> ***************
> *** 1411,1416 ****
> --- 1411,1424 ----
>   	AfterTriggerBeginXact();
>   
>   	/*
> + 	 * mark the transaction as not VACUUM  (vacuum_rel will set isVacuum to true 
> + 	 * directly after calling BeginTransactionCommand() )
> + 	 */
> + 	if (MyProc != NULL)  
> + 	{
> + 		MyProc->inVacuum = false;
> + 	}

I'm a little worried about having this set to "true" after a VACUUM is 
executed, and only reset to false when the next transaction is begun: it 
shouldn't affect correctness right now, but it seems like asking for 
trouble. Resetting the flag to "false" after processing a transaction 
would probably be worth doing.

> *** src/backend/commands/vacuum.c	6 May 2005 17:24:53 -0000	1.308
> --- src/backend/commands/vacuum.c	17 May 2005 22:06:35 -0000
> ***************
> *** 420,425 ****
> --- 418,428 ----
>   				if (use_own_xacts)
>   				{
>   					StartTransactionCommand();
> + 					if (MyProc != NULL)  /* is this needed here ? */
> + 					{
> + 						/* so other vacuums don't look at our xid/xmin in GetOldestXmin() */
> + 						MyProc->inVacuum = true;
> + 					}
>   					/* functions in indexes may want a snapshot set */
>   					ActiveSnapshot = CopySnapshot(GetTransactionSnapshot());
>   				}

Is it valid to apply this optimization to ANALYZE? Since ANALYZE updates 
pg_statistic, ISTM it can affect tuple visibility.

> *** src/backend/storage/ipc/sinval.c	31 Dec 2004 22:00:56 -0000	1.75
> --- src/backend/storage/ipc/sinval.c	17 May 2005 22:06:36 -0000
> ***************
> *** 845,854 ****
>   			 * them as running anyway.	We also assume that such xacts
>   			 * can't compute an xmin older than ours, so they needn't be
>   			 * considered in computing globalxmin.
>   			 */
>   			if (proc == MyProc ||
>   				!TransactionIdIsNormal(xid) ||
> ! 				TransactionIdFollowsOrEquals(xid, xmax))
>   				continue;
>   
>   			if (TransactionIdPrecedes(xid, xmin))
> --- 845,858 ----
>   			 * them as running anyway.	We also assume that such xacts
>   			 * can't compute an xmin older than ours, so they needn't be
>   			 * considered in computing globalxmin.
> + 			 *
> + 			 * there is also no need to consider transaxtions runnibg the 
> + 			 * vacuum command as it will not affect tuple visibility
>   			 */

Typos.

-Neil