Tom Lane wrote:
> Alvaro Herrera <alvhe...@commandprompt.com> writes:
> > Tom Lane wrote:
> >> This just seems truly messy :-(.  Let me see if I can find something
> >> cleaner.
> 
> > I was considering having InitPostgres be an umbrella function, so that
> > extant callers stay as today, but the various underlying pieces are
> > skipped depending on who's calling.  For example I didn't like the bit
> > about starting a transaction or not depending on whether it was the
> > launcher.
> 
> Yeah.  If you have InitPostgres know that much about the AV launcher's
> requirements, it's not clear why it shouldn't just know everything.
> Having it return with the initial transaction still open just seems
> completely horrid.

How about this?

> While I was looking at this I wondered whether
> RelationCacheInitializePhase2 really needs to be inside the startup
> transaction at all.  I think it could probably be moved up before
> that.  However, if the AV launcher has to do GetTransactionSnapshot
> then it's not clear that improves matters anyway. 

Well, the difference is that the initial transaction would be a few
microsec shorter ... not sure if that matters.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/postmaster/autovacuum.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.103
diff -c -p -r1.103 autovacuum.c
*** src/backend/postmaster/autovacuum.c	27 Aug 2009 17:18:44 -0000	1.103
--- src/backend/postmaster/autovacuum.c	31 Aug 2009 15:49:14 -0000
*************** AutoVacLauncherMain(int argc, char *argv
*** 424,432 ****
  #endif
  
  	/*
! 	 * Set up signal handlers.	Since this is an auxiliary process, it has
! 	 * particular signal requirements -- no deadlock checker or sinval
! 	 * catchup, for example.
  	 */
  	pqsignal(SIGHUP, avl_sighup_handler);
  
--- 424,432 ----
  #endif
  
  	/*
! 	 * Set up signal handlers.	We operate on databases much like a regular
! 	 * backend, so we use the same signal handling.  See equivalent code in
! 	 * tcop/postgres.c.
  	 */
  	pqsignal(SIGHUP, avl_sighup_handler);
  
*************** AutoVacLauncherMain(int argc, char *argv
*** 451,459 ****
  	 * had to do some stuff with LWLocks).
  	 */
  #ifndef EXEC_BACKEND
! 	InitAuxiliaryProcess();
  #endif
  
  	/*
  	 * Create a memory context that we will do all our work in.  We do this so
  	 * that we can reset the context during error recovery and thereby avoid
--- 451,461 ----
  	 * had to do some stuff with LWLocks).
  	 */
  #ifndef EXEC_BACKEND
! 	InitProcess();
  #endif
  
+ 	InitPostgres(NULL, InvalidOid, NULL, NULL);
+ 
  	/*
  	 * Create a memory context that we will do all our work in.  We do this so
  	 * that we can reset the context during error recovery and thereby avoid
*************** AutoVacLauncherMain(int argc, char *argv
*** 470,476 ****
  	/*
  	 * If an exception is encountered, processing resumes here.
  	 *
! 	 * This code is heavily based on bgwriter.c, q.v.
  	 */
  	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
  	{
--- 472,478 ----
  	/*
  	 * If an exception is encountered, processing resumes here.
  	 *
! 	 * This code is a stripped down version of PostgresMain error recovery.
  	 */
  	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
  	{
*************** AutoVacLauncherMain(int argc, char *argv
*** 483,496 ****
  		/* Report the error to the server log */
  		EmitErrorReport();
  
! 		/*
! 		 * These operations are really just a minimal subset of
! 		 * AbortTransaction().	We don't have very many resources to worry
! 		 * about, but we do have LWLocks.
! 		 */
! 		LWLockReleaseAll();
! 		AtEOXact_Files();
! 		AtEOXact_HashTables(false);
  
  		/*
  		 * Now return to normal top-level context and clear ErrorContext for
--- 485,492 ----
  		/* Report the error to the server log */
  		EmitErrorReport();
  
! 		/* Abort the current transaction in order to recover */
! 		AbortCurrentTransaction();
  
  		/*
  		 * Now return to normal top-level context and clear ErrorContext for
*************** autovac_balance_cost(void)
*** 1784,1829 ****
  
  /*
   * get_database_list
   *
!  *		Return a list of all databases.  Note we cannot use pg_database,
!  *		because we aren't connected; we use the flat database file.
   */
  static List *
  get_database_list(void)
  {
- 	char	   *filename;
  	List	   *dblist = NIL;
! 	char		thisname[NAMEDATALEN];
! 	FILE	   *db_file;
! 	Oid			db_id;
! 	Oid			db_tablespace;
! 	TransactionId db_frozenxid;
! 
! 	filename = database_getflatfilename();
! 	db_file = AllocateFile(filename, "r");
! 	if (db_file == NULL)
! 		ereport(FATAL,
! 				(errcode_for_file_access(),
! 				 errmsg("could not open file \"%s\": %m", filename)));
  
! 	while (read_pg_database_line(db_file, thisname, &db_id,
! 								 &db_tablespace, &db_frozenxid))
  	{
! 		avw_dbase  *avdb;
  
  		avdb = (avw_dbase *) palloc(sizeof(avw_dbase));
  
! 		avdb->adw_datid = db_id;
! 		avdb->adw_name = pstrdup(thisname);
! 		avdb->adw_frozenxid = db_frozenxid;
  		/* this gets set later: */
  		avdb->adw_entry = NULL;
  
  		dblist = lappend(dblist, avdb);
  	}
  
! 	FreeFile(db_file);
! 	pfree(filename);
  
  	return dblist;
  }
--- 1780,1825 ----
  
  /*
   * get_database_list
+  *		Return a list of all databases found in pg_database.
   *
!  * Note: this is the only function in which the autovacuum launcher uses a
!  * transaction.
   */
  static List *
  get_database_list(void)
  {
  	List	   *dblist = NIL;
! 	Relation	rel;
! 	HeapScanDesc scan;
! 	HeapTuple	tup;
  
! 	StartTransactionCommand();
! 
! 	MemoryContextSwitchTo(AutovacMemCxt);
! 
! 	rel = heap_open(DatabaseRelationId, AccessShareLock);
! 	scan = heap_beginscan(rel, SnapshotNow, 0, NULL);
! 
! 	while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))
  	{
! 		Form_pg_database pgdatabase = (Form_pg_database) GETSTRUCT(tup);
! 		avw_dbase   *avdb;
  
  		avdb = (avw_dbase *) palloc(sizeof(avw_dbase));
  
! 		avdb->adw_datid = HeapTupleGetOid(tup);
! 		avdb->adw_name = pstrdup(NameStr(pgdatabase->datname));
! 		avdb->adw_frozenxid = pgdatabase->datfrozenxid;
  		/* this gets set later: */
  		avdb->adw_entry = NULL;
  
  		dblist = lappend(dblist, avdb);
  	}
  
! 	heap_endscan(scan);
! 	heap_close(rel, AccessShareLock);
! 
! 	CommitTransactionCommand();
  
  	return dblist;
  }
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.593
diff -c -p -r1.593 postmaster.c
*** src/backend/postmaster/postmaster.c	29 Aug 2009 19:26:51 -0000	1.593
--- src/backend/postmaster/postmaster.c	31 Aug 2009 13:37:29 -0000
*************** SubPostmasterMain(int argc, char *argv[]
*** 3865,3871 ****
  		InitShmemAccess(UsedShmemSegAddr);
  
  		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
! 		InitAuxiliaryProcess();
  
  		/* Attach process to shared data structures */
  		CreateSharedMemoryAndSemaphores(false, 0);
--- 3865,3871 ----
  		InitShmemAccess(UsedShmemSegAddr);
  
  		/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
! 		InitProcess();
  
  		/* Attach process to shared data structures */
  		CreateSharedMemoryAndSemaphores(false, 0);
Index: src/backend/storage/lmgr/proc.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/lmgr/proc.c,v
retrieving revision 1.208
diff -c -p -r1.208 proc.c
*** src/backend/storage/lmgr/proc.c	12 Aug 2009 20:53:30 -0000	1.208
--- src/backend/storage/lmgr/proc.c	31 Aug 2009 13:36:56 -0000
*************** ProcGlobalShmemSize(void)
*** 102,109 ****
  	size = add_size(size, sizeof(PROC_HDR));
  	/* AuxiliaryProcs */
  	size = add_size(size, mul_size(NUM_AUXILIARY_PROCS, sizeof(PGPROC)));
! 	/* MyProcs, including autovacuum */
! 	size = add_size(size, mul_size(MaxBackends, sizeof(PGPROC)));
  	/* ProcStructLock */
  	size = add_size(size, sizeof(slock_t));
  
--- 102,109 ----
  	size = add_size(size, sizeof(PROC_HDR));
  	/* AuxiliaryProcs */
  	size = add_size(size, mul_size(NUM_AUXILIARY_PROCS, sizeof(PGPROC)));
! 	/* MyProcs, including autovacuum workers and launcher */
! 	size = add_size(size, mul_size(MaxBackends + 1, sizeof(PGPROC)));
  	/* ProcStructLock */
  	size = add_size(size, sizeof(slock_t));
  
*************** InitProcGlobal(void)
*** 192,204 ****
  		ProcGlobal->freeProcs = &procs[i];
  	}
  
! 	procs = (PGPROC *) ShmemAlloc((autovacuum_max_workers) * sizeof(PGPROC));
  	if (!procs)
  		ereport(FATAL,
  				(errcode(ERRCODE_OUT_OF_MEMORY),
  				 errmsg("out of shared memory")));
! 	MemSet(procs, 0, autovacuum_max_workers * sizeof(PGPROC));
! 	for (i = 0; i < autovacuum_max_workers; i++)
  	{
  		PGSemaphoreCreate(&(procs[i].sem));
  		procs[i].links.next = (SHM_QUEUE *) ProcGlobal->autovacFreeProcs;
--- 192,204 ----
  		ProcGlobal->freeProcs = &procs[i];
  	}
  
! 	procs = (PGPROC *) ShmemAlloc((autovacuum_max_workers + 1) * sizeof(PGPROC));
  	if (!procs)
  		ereport(FATAL,
  				(errcode(ERRCODE_OUT_OF_MEMORY),
  				 errmsg("out of shared memory")));
! 	MemSet(procs, 0, (autovacuum_max_workers + 1) * sizeof(PGPROC));
! 	for (i = 0; i < autovacuum_max_workers + 1; i++)
  	{
  		PGSemaphoreCreate(&(procs[i].sem));
  		procs[i].links.next = (SHM_QUEUE *) ProcGlobal->autovacFreeProcs;
*************** InitProcess(void)
*** 248,261 ****
  
  	set_spins_per_delay(procglobal->spins_per_delay);
  
! 	if (IsAutoVacuumWorkerProcess())
  		MyProc = procglobal->autovacFreeProcs;
  	else
  		MyProc = procglobal->freeProcs;
  
  	if (MyProc != NULL)
  	{
! 		if (IsAutoVacuumWorkerProcess())
  			procglobal->autovacFreeProcs = (PGPROC *) MyProc->links.next;
  		else
  			procglobal->freeProcs = (PGPROC *) MyProc->links.next;
--- 248,261 ----
  
  	set_spins_per_delay(procglobal->spins_per_delay);
  
! 	if (IsAnyAutoVacuumProcess())
  		MyProc = procglobal->autovacFreeProcs;
  	else
  		MyProc = procglobal->freeProcs;
  
  	if (MyProc != NULL)
  	{
! 		if (IsAnyAutoVacuumProcess())
  			procglobal->autovacFreeProcs = (PGPROC *) MyProc->links.next;
  		else
  			procglobal->freeProcs = (PGPROC *) MyProc->links.next;
*************** InitProcess(void)
*** 280,286 ****
  	 * child; this is so that the postmaster can detect it if we exit without
  	 * cleaning up.
  	 */
! 	if (IsUnderPostmaster)
  		MarkPostmasterChildActive();
  
  	/*
--- 280,286 ----
  	 * child; this is so that the postmaster can detect it if we exit without
  	 * cleaning up.
  	 */
! 	if (IsUnderPostmaster && !IsAutoVacuumLauncherProcess())
  		MarkPostmasterChildActive();
  
  	/*
*************** InitProcess(void)
*** 299,304 ****
--- 299,305 ----
  	MyProc->roleId = InvalidOid;
  	MyProc->inCommit = false;
  	MyProc->vacuumFlags = 0;
+ 	/* NB -- autovac launcher does not set this flag to avoid getting killed */
  	if (IsAutoVacuumWorkerProcess())
  		MyProc->vacuumFlags |= PROC_IS_AUTOVACUUM;
  	MyProc->lwWaiting = false;
*************** InitAuxiliaryProcess(void)
*** 429,435 ****
  	MyProc->databaseId = InvalidOid;
  	MyProc->roleId = InvalidOid;
  	MyProc->inCommit = false;
- 	/* we don't set the "is autovacuum" flag in the launcher */
  	MyProc->vacuumFlags = 0;
  	MyProc->lwWaiting = false;
  	MyProc->lwExclusive = false;
--- 430,435 ----
*************** ProcKill(int code, Datum arg)
*** 596,602 ****
  	SpinLockAcquire(ProcStructLock);
  
  	/* Return PGPROC structure (and semaphore) to freelist */
! 	if (IsAutoVacuumWorkerProcess())
  	{
  		MyProc->links.next = (SHM_QUEUE *) procglobal->autovacFreeProcs;
  		procglobal->autovacFreeProcs = MyProc;
--- 596,602 ----
  	SpinLockAcquire(ProcStructLock);
  
  	/* Return PGPROC structure (and semaphore) to freelist */
! 	if (IsAnyAutoVacuumProcess())
  	{
  		MyProc->links.next = (SHM_QUEUE *) procglobal->autovacFreeProcs;
  		procglobal->autovacFreeProcs = MyProc;
*************** ProcKill(int code, Datum arg)
*** 619,625 ****
  	 * This process is no longer present in shared memory in any meaningful
  	 * way, so tell the postmaster we've cleaned up acceptably well.
  	 */
! 	if (IsUnderPostmaster)
  		MarkPostmasterChildInactive();
  
  	/* wake autovac launcher if needed -- see comments in FreeWorkerInfo */
--- 619,625 ----
  	 * This process is no longer present in shared memory in any meaningful
  	 * way, so tell the postmaster we've cleaned up acceptably well.
  	 */
! 	if (IsUnderPostmaster && !IsAutoVacuumLauncherProcess())
  		MarkPostmasterChildInactive();
  
  	/* wake autovac launcher if needed -- see comments in FreeWorkerInfo */
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.195
diff -c -p -r1.195 postinit.c
*** src/backend/utils/init/postinit.c	29 Aug 2009 19:26:51 -0000	1.195
--- src/backend/utils/init/postinit.c	31 Aug 2009 15:48:10 -0000
*************** InitPostgres(const char *in_dbname, Oid 
*** 552,557 ****
--- 552,560 ----
  	 * Start a new transaction here before first access to db, and get a
  	 * snapshot.  We don't have a use for the snapshot itself, but we're
  	 * interested in the secondary effect that it sets RecentGlobalXmin.
+ 	 * This is critical for anything that reads heap pages, because HOT
+ 	 * may decide to prune them even if the process doesn't attempt to
+ 	 * modify it.
  	 */
  	if (!bootstrap)
  	{
*************** InitPostgres(const char *in_dbname, Oid 
*** 565,570 ****
--- 568,577 ----
  	 */
  	RelationCacheInitializePhase2();
  
+ 	/* The autovacuum launcher is done here */
+ 	if (IsAutoVacuumLauncherProcess())
+ 		goto done;
+ 
  	/*
  	 * Set up the global variables holding database id and default tablespace.
  	 * But note we won't actually try to touch the database just yet.
*************** InitPostgres(const char *in_dbname, Oid 
*** 774,779 ****
--- 781,787 ----
  	if (!bootstrap)
  		pgstat_bestart();
  
+ done:
  	/* close the transaction we started above */
  	if (!bootstrap)
  		CommitTransactionCommand();
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.513
diff -c -p -r1.513 guc.c
*** src/backend/utils/misc/guc.c	31 Aug 2009 02:23:22 -0000	1.513
--- src/backend/utils/misc/guc.c	31 Aug 2009 03:07:47 -0000
*************** static struct config_int ConfigureNamesI
*** 1335,1341 ****
  	 * Note: MaxBackends is limited to INT_MAX/4 because some places compute
  	 * 4*MaxBackends without any overflow check.  This check is made in
  	 * assign_maxconnections, since MaxBackends is computed as MaxConnections
! 	 * plus autovacuum_max_workers.
  	 *
  	 * Likewise we have to limit NBuffers to INT_MAX/2.
  	 */
--- 1335,1341 ----
  	 * Note: MaxBackends is limited to INT_MAX/4 because some places compute
  	 * 4*MaxBackends without any overflow check.  This check is made in
  	 * assign_maxconnections, since MaxBackends is computed as MaxConnections
! 	 * plus autovacuum_max_workers plus one (for the autovacuum launcher).
  	 *
  	 * Likewise we have to limit NBuffers to INT_MAX/2.
  	 */
*************** assign_maxconnections(int newval, bool d
*** 7574,7580 ****
  		return false;
  
  	if (doit)
! 		MaxBackends = newval + autovacuum_max_workers;
  
  	return true;
  }
--- 7574,7580 ----
  		return false;
  
  	if (doit)
! 		MaxBackends = newval + autovacuum_max_workers + 1;
  
  	return true;
  }
*************** assign_autovacuum_max_workers(int newval
*** 7586,7592 ****
  		return false;
  
  	if (doit)
! 		MaxBackends = newval + MaxConnections;
  
  	return true;
  }
--- 7586,7592 ----
  		return false;
  
  	if (doit)
! 		MaxBackends = newval + MaxConnections + 1;
  
  	return true;
  }
Index: src/include/postmaster/autovacuum.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/postmaster/autovacuum.h,v
retrieving revision 1.15
diff -c -p -r1.15 autovacuum.h
*** src/include/postmaster/autovacuum.h	1 Jan 2009 17:24:01 -0000	1.15
--- src/include/postmaster/autovacuum.h	31 Aug 2009 03:29:17 -0000
*************** extern int	Log_autovacuum_min_duration;
*** 37,42 ****
--- 37,44 ----
  extern bool AutoVacuumingActive(void);
  extern bool IsAutoVacuumLauncherProcess(void);
  extern bool IsAutoVacuumWorkerProcess(void);
+ #define IsAnyAutoVacuumProcess() (IsAutoVacuumLauncherProcess() || \
+ 								  IsAutoVacuumWorkerProcess())
  
  /* Functions to start autovacuum process, called from postmaster */
  extern void autovac_init(void);
Index: src/include/storage/proc.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/storage/proc.h,v
retrieving revision 1.113
diff -c -p -r1.113 proc.h
*** src/include/storage/proc.h	12 Aug 2009 20:53:30 -0000	1.113
--- src/include/storage/proc.h	31 Aug 2009 03:08:42 -0000
*************** typedef struct PROC_HDR
*** 141,152 ****
   * We set aside some extra PGPROC structures for auxiliary processes,
   * ie things that aren't full-fledged backends but need shmem access.
   *
!  * Background writer, WAL writer, and autovacuum launcher run during
!  * normal operation. Startup process also consumes one slot, but WAL
!  * writer and autovacuum launcher are launched only after it has
   * exited.
   */
! #define NUM_AUXILIARY_PROCS		3
  
  
  /* configurable options */
--- 141,151 ----
   * We set aside some extra PGPROC structures for auxiliary processes,
   * ie things that aren't full-fledged backends but need shmem access.
   *
!  * Background writer and WAL writer run during normal operation. Startup
!  * process also consumes one slot, but WAL writer is launched only after it has
   * exited.
   */
! #define NUM_AUXILIARY_PROCS		2
  
  
  /* configurable options */
-- 
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