Tom Lane wrote: > Alvaro Herrera <alvhe...@commandprompt.com> writes: > > How about this? > > I think the accounting for the AV launcher in shmem allocation is a bit > confused yet --- for instance, isn't MaxBackends already including the > launcher? I wonder if it would be cleaner to include the launcher in > the autovacuum_max_workers parameter, and increase the min/default > values of that by one.
Huh, yeah, sorry about that -- fixed here. I think the name of the param, which includes "worker", precludes from raising the values. Changes between v2 and v3: diff -u src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c --- src/backend/storage/lmgr/proc.c 31 Aug 2009 13:36:56 -0000 +++ src/backend/storage/lmgr/proc.c 31 Aug 2009 16:14:08 -0000 @@ -103,7 +103,7 @@ /* 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))); + size = add_size(size, mul_size(MaxBackends, sizeof(PGPROC))); /* ProcStructLock */ size = add_size(size, sizeof(slock_t)); @@ -192,6 +192,7 @@ ProcGlobal->freeProcs = &procs[i]; } + /* note: the "+1" here accounts for the autovac launcher */ procs = (PGPROC *) ShmemAlloc((autovacuum_max_workers + 1) * sizeof(PGPROC)); if (!procs) ereport(FATAL, diff -u src/backend/utils/misc/guc.c src/backend/utils/misc/guc.c --- src/backend/utils/misc/guc.c 31 Aug 2009 03:07:47 -0000 +++ src/backend/utils/misc/guc.c 31 Aug 2009 16:12:56 -0000 @@ -7570,7 +7570,7 @@ static bool assign_maxconnections(int newval, bool doit, GucSource source) { - if (newval + autovacuum_max_workers > INT_MAX / 4) + if (newval + autovacuum_max_workers + 1 > INT_MAX / 4) return false; if (doit) -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
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 16:14:08 -0000 *************** ProcGlobalShmemSize(void) *** 102,108 **** 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,108 ---- 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, 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,205 ---- ProcGlobal->freeProcs = &procs[i]; } ! /* note: the "+1" here accounts for the autovac launcher */ ! 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; --- 249,262 ---- 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(); /* --- 281,287 ---- * child; this is so that the postmaster can detect it if we exit without * cleaning up. */ ! if (IsUnderPostmaster && !IsAutoVacuumLauncherProcess()) MarkPostmasterChildActive(); /* *************** InitProcess(void) *** 299,304 **** --- 300,306 ---- 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; --- 431,436 ---- *************** 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; --- 597,603 ---- 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 */ --- 620,626 ---- * 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 16:12:56 -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. */ *************** show_tcp_keepalives_count(void) *** 7570,7580 **** static bool assign_maxconnections(int newval, bool doit, GucSource source) { ! if (newval + autovacuum_max_workers > INT_MAX / 4) return false; if (doit) ! MaxBackends = newval + autovacuum_max_workers; return true; } --- 7570,7580 ---- static bool assign_maxconnections(int newval, bool doit, GucSource source) { ! if (newval + autovacuum_max_workers + 1 > INT_MAX / 4) 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