On E, 2005-05-23 at 11:42 -0400, Tom Lane wrote:
> Hannu Krosing <[EMAIL PROTECTED]> writes:
> > I can't think of any other cases where it could matter, as at least the
> > work done inside vacuum_rel() itself seema non-rollbackable.
> 
> VACUUM FULL's tuple-moving is definitely roll-back-able, so it might be
> prudent to only do this for lazy VACUUM.  But on the other hand, VACUUM
> FULL holds an exclusive lock on the table so no one else is going to see
> its effects concurrently anyway.

Ok, this is a new version of the vacuum patch with the following changes
following some suggestions in this thread.

* changed the patch to affect only lazy vacuum 
* moved inVacuum handling to use PG_TRY
* moved vac_update_relstats() out of lazy_vacuum_rel into a separate
  transaction. The code to do this may not be the prettiest, maybe it
  should use a separate struct.

-- 
Hannu Krosing <[EMAIL PROTECTED]>
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.209
diff -c -r1.209 xact.c
*** src/backend/access/transam/xact.c	29 Jun 2005 22:51:53 -0000	1.209
--- src/backend/access/transam/xact.c	3 Jul 2005 15:59:09 -0000
***************
*** 1402,1407 ****
--- 1402,1416 ----
  	AfterTriggerBeginXact();
  
  	/*
+ 	 * mark the transaction as not VACUUM  (vacuum_rel will set isVacuum 
+ 	 * to true directly after calling BeginTransactionCommand() )
+ 	 *
+ 	 * this can probably be moved to be done only once when establishing 
+ 	 * connection as this is now quaranteedto be reset to false in vacuum.c
+ 	 */
+ 	MyProc->inVacuum = false;
+ 
+ 	/*
  	 * done with start processing, set current transaction state to "in
  	 * progress"
  	 */
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.310
diff -c -r1.310 vacuum.c
*** src/backend/commands/vacuum.c	14 Jun 2005 22:15:32 -0000	1.310
--- src/backend/commands/vacuum.c	3 Jul 2005 15:59:15 -0000
***************
*** 37,42 ****
--- 37,43 ----
  #include "miscadmin.h"
  #include "storage/freespace.h"
  #include "storage/procarray.h"
+ #include "storage/proc.h"
  #include "storage/smgr.h"
  #include "tcop/pquery.h"
  #include "utils/acl.h"
***************
*** 903,908 ****
--- 904,913 ----
  	Oid			toast_relid;
  	bool		result;
  
+ 	BlockNumber	stats_rel_pages=0;
+ 	double		stats_rel_tuples=0;
+ 	bool		stats_hasindex=false;
+ 
  	/* Begin a transaction for vacuuming this relation */
  	StartTransactionCommand();
  	/* functions in indexes may want a snapshot set */
***************
*** 1016,1039 ****
  	 */
  	toast_relid = onerel->rd_rel->reltoastrelid;
  
! 	/*
! 	 * Do the actual work --- either FULL or "lazy" vacuum
! 	 */
! 	if (vacstmt->full)
! 		full_vacuum_rel(onerel, vacstmt);
! 	else
! 		lazy_vacuum_rel(onerel, vacstmt);
  
! 	result = true;				/* did the vacuum */
  
! 	/* all done with this class, but hold lock until commit */
! 	relation_close(onerel, NoLock);
  
! 	/*
! 	 * Complete the transaction and free all temporary memory used.
! 	 */
! 	StrategyHintVacuum(false);
! 	CommitTransactionCommand();
  
  	/*
  	 * If the relation has a secondary toast rel, vacuum that too while we
--- 1021,1071 ----
  	 */
  	toast_relid = onerel->rd_rel->reltoastrelid;
  
! 	PG_TRY();
! 	{
! 		/*
! 		 * Do the actual work --- either FULL or "lazy" vacuum
! 		 */
! 		if (vacstmt->full)
! 			full_vacuum_rel(onerel, vacstmt);
! 		else
! 			/* mark this transaction as being a lazy vacuum */
! 			MyProc->inVacuum = true;
  
! 			lazy_vacuum_rel(onerel, vacstmt, &stats_rel_pages, &stats_rel_tuples, &stats_hasindex);
  
! 		result = true;				/* did the vacuum */
  
! 		/* all done with this class, but hold lock until commit */
! 		relation_close(onerel, NoLock);
! 
! 		/*
! 		 * Complete the transaction and free all temporary memory used.
! 		 */
! 		StrategyHintVacuum(false);
! 		CommitTransactionCommand();
! 	}
! 	PG_CATCH();
! 	{
! 		/* make sure in-vacuum flag is cleared is cleared */
! 		MyProc->inVacuum = false;
! 		PG_RE_THROW();
! 	}
! 	PG_END_TRY();
! 	MyProc->inVacuum = false;
! 
! 
!     /* use yet another transaction for saving stats from lazy_vacuum_rel into pg_class */
!     if (stats_rel_pages > 0) {
! 		StartTransactionCommand();
! 		ActiveSnapshot = CopySnapshot(GetTransactionSnapshot()); /* is this needed ? */		
! 		/* Update statistics in pg_class */
! 		vac_update_relstats(RelationGetRelid(onerel),
! 							stats_rel_pages,
! 							stats_rel_tuples,
! 							stats_hasindex);
! 		CommitTransactionCommand();
! 	}
  
  	/*
  	 * If the relation has a secondary toast rel, vacuum that too while we
Index: src/backend/commands/vacuumlazy.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v
retrieving revision 1.54
diff -c -r1.54 vacuumlazy.c
*** src/backend/commands/vacuumlazy.c	19 May 2005 21:35:46 -0000	1.54
--- src/backend/commands/vacuumlazy.c	3 Jul 2005 15:59:17 -0000
***************
*** 128,134 ****
   *		and locked the relation.
   */
  void
! lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
  {
  	LVRelStats *vacrelstats;
  	Relation   *Irel;
--- 128,134 ----
   *		and locked the relation.
   */
  void
! lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, BlockNumber *stats_rel_pages, double *stats_rel_tuples, bool *stats_hasindex)
  {
  	LVRelStats *vacrelstats;
  	Relation   *Irel;
***************
*** 175,184 ****
--- 175,190 ----
  	lazy_update_fsm(onerel, vacrelstats);
  
  	/* Update statistics in pg_class */
+ 	/*
  	vac_update_relstats(RelationGetRelid(onerel),
  						vacrelstats->rel_pages,
  						vacrelstats->rel_tuples,
  						hasindex);
+ 	*/
+ 	/* return values for setting relstats in another transaction */
+ 	*stats_rel_pages = vacrelstats->rel_pages;
+ 	*stats_rel_tuples = vacrelstats->rel_tuples;
+ 	*stats_hasindex = hasindex;
  }
  
  
Index: src/backend/storage/ipc/procarray.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/ipc/procarray.c,v
retrieving revision 1.3
diff -c -r1.3 procarray.c
*** src/backend/storage/ipc/procarray.c	17 Jun 2005 22:32:45 -0000	1.3
--- src/backend/storage/ipc/procarray.c	3 Jul 2005 15:59:17 -0000
***************
*** 423,429 ****
  	{
  		PGPROC	   *proc = arrayP->procs[index];
  
! 		if (allDbs || proc->databaseId == MyDatabaseId)
  		{
  			/* Fetch xid just once - see GetNewTransactionId */
  			TransactionId xid = proc->xid;
--- 423,431 ----
  	{
  		PGPROC	   *proc = arrayP->procs[index];
  
! 		/* don't use xid of transactions running lazy vacuum */
! 		if (   (proc->inVacuum == false) 
! 			&& (allDbs || proc->databaseId == MyDatabaseId))
  		{
  			/* Fetch xid just once - see GetNewTransactionId */
  			TransactionId xid = proc->xid;
Index: src/include/commands/vacuum.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/commands/vacuum.h,v
retrieving revision 1.59
diff -c -r1.59 vacuum.h
*** src/include/commands/vacuum.h	31 Dec 2004 22:03:28 -0000	1.59
--- src/include/commands/vacuum.h	3 Jul 2005 15:59:17 -0000
***************
*** 142,148 ****
  extern void vacuum_delay_point(void);
  
  /* in commands/vacuumlazy.c */
! extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt);
  
  /* in commands/analyze.c */
  extern void analyze_rel(Oid relid, VacuumStmt *vacstmt);
--- 142,151 ----
  extern void vacuum_delay_point(void);
  
  /* in commands/vacuumlazy.c */
! extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
! 							BlockNumber *stats_rel_pages, 
! 							double *stats_rel_tuples, 
! 							bool *stats_hasindex);
  
  /* in commands/analyze.c */
  extern void analyze_rel(Oid relid, VacuumStmt *vacstmt);
Index: src/include/storage/proc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/storage/proc.h,v
retrieving revision 1.79
diff -c -r1.79 proc.h
*** src/include/storage/proc.h	17 Jun 2005 22:32:50 -0000	1.79
--- src/include/storage/proc.h	3 Jul 2005 15:59:18 -0000
***************
*** 69,74 ****
--- 69,80 ----
  								 * were starting our xact: vacuum must not
  								 * remove tuples deleted by xid >= xmin ! */
  
+ 	bool		inVacuum;       /* true if current command is vacuum.
+ 								 * used by other vacuum commands 
+ 								 * as xid or xmin of this cmd need not be used 
+ 								 * when finding global xmin for removing 
+ 								 * tuples */
+ 
  	int			pid;			/* This backend's process id, or 0 */
  	Oid			databaseId;		/* OID of database this backend is using */
  
---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
       choose an index scan if your joining column's datatypes do not
       match

Reply via email to