I committed background workers three weeks ago, claiming it worked on
EXEC_BACKEND, and shortly thereafter I discovered that it didn't. I
noticed that the problem is the kludge to cause postmaster and children
to recompute MaxBackends after shared_preload_libraries is processed; so
the minimal fix is to duplicate this bit, from PostmasterMain() into
SubPostmasterMain():
@@ -4443,6 +4443,17 @@ SubPostmasterMain(int argc, char *argv[])
*/
process_shared_preload_libraries();
+ /*
+ * If loadable modules have added background workers, MaxBackends needs to
+ * be updated. Do so now by forcing a no-op update of max_connections.
+ * XXX This is a pretty ugly way to do it, but it doesn't seem worth
+ * introducing a new entry point in guc.c to do it in a cleaner fashion.
+ */
+ if (GetNumShmemAttachedBgworkers() > 0)
+ SetConfigOption("max_connections",
+ GetConfigOption("max_connections", false, false),
+ PGC_POSTMASTER, PGC_S_OVERRIDE);
I considered this pretty ugly when I first wrote it, and as the comment
says I tried to add something to guc.c to make it cleaner, but it was
even uglier.
So I now came up with a completely different idea: how about making
MaxBackends a macro, i.e.
+#define MaxBackends (MaxConnections + autovacuum_max_workers + 1 + \
+ GetNumShmemAttachedBgworkers())
so that instead of having guc.c recompute it, each caller that needs to
value obtains it up to date all the time? This additionally means that
assign_maxconnections and assign_autovacuum_max_workers go away (only
the check routines remain). Patch attached.
The one problem I see as serious with this approach is that it'd be
moderately expensive (i.e. not just fetch a value from memory) to
compute the value because it requires a walk of the registered workers
list. For most callers this wouldn't be a problem because it's just
during shmem sizing/creation; but there are places such as multixact.c
and async.c that use it routinely, so it's likely that we need to cache
the value somehow. It seems relatively straightforward though.
I'd like to hear opinions on just staying with the IMO ugly minimal fix,
or pursue instead making MaxBackends a macro plus some sort of cache to
avoid repeated computation.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/access/nbtree/nbtutils.c
--- b/src/backend/access/nbtree/nbtutils.c
***************
*** 21,26 ****
--- 21,27 ----
#include "access/reloptions.h"
#include "access/relscan.h"
#include "miscadmin.h"
+ #include "storage/proc.h"
#include "utils/array.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
*** a/src/backend/access/transam/multixact.c
--- b/src/backend/access/transam/multixact.c
***************
*** 57,62 ****
--- 57,63 ----
#include "miscadmin.h"
#include "pg_trace.h"
#include "storage/lmgr.h"
+ #include "storage/proc.h"
#include "storage/procarray.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
*** a/src/backend/commands/async.c
--- b/src/backend/commands/async.c
***************
*** 126,131 ****
--- 126,132 ----
#include "miscadmin.h"
#include "storage/ipc.h"
#include "storage/lmgr.h"
+ #include "storage/proc.h"
#include "storage/procsignal.h"
#include "storage/sinval.h"
#include "tcop/tcopprot.h"
*** a/src/backend/libpq/pqcomm.c
--- b/src/backend/libpq/pqcomm.c
***************
*** 93,98 ****
--- 93,99 ----
#include "libpq/libpq.h"
#include "miscadmin.h"
#include "storage/ipc.h"
+ #include "storage/proc.h"
#include "utils/guc.h"
#include "utils/memutils.h"
*** a/src/backend/postmaster/autovacuum.c
--- b/src/backend/postmaster/autovacuum.c
***************
*** 108,114 ****
* GUC parameters
*/
bool autovacuum_start_daemon = false;
! int autovacuum_max_workers;
int autovacuum_naptime;
int autovacuum_vac_thresh;
double autovacuum_vac_scale;
--- 108,114 ----
* GUC parameters
*/
bool autovacuum_start_daemon = false;
! /* autovacuum_max_workers is elsewhere */
int autovacuum_naptime;
int autovacuum_vac_thresh;
double autovacuum_vac_scale;
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***************
*** 52,57 ****
--- 52,58 ----
#include "storage/ipc.h"
#include "storage/latch.h"
#include "storage/pg_shmem.h"
+ #include "storage/proc.h"
#include "storage/procsignal.h"
#include "utils/ascii.h"
#include "utils/guc.h"
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***************
*** 897,913 **** PostmasterMain(int argc, char *argv[])
process_shared_preload_libraries();
/*
- * If loadable modules have added background workers, MaxBackends needs to
- * be updated. Do so now by forcing a no-op update of max_connections.
- * XXX This is a pretty ugly way to do it, but it doesn't seem worth
- * introducing a new entry point in guc.c to do it in a cleaner fashion.
- */
- if (GetNumShmemAttachedBgworkers() > 0)
- SetConfigOption("max_connections",
- GetConfigOption("max_connections", false, false),
- PGC_POSTMASTER, PGC_S_OVERRIDE);
-
- /*
* Establish input sockets.
*/
for (i = 0; i < MAXLISTEN; i++)
--- 897,902 ----
***************
*** 5176,5181 **** RegisterBackgroundWorker(BackgroundWorker *worker)
--- 5165,5180 ----
return;
}
+ if (MaxBackends >= MAX_BACKENDS)
+ {
+ ereport(LOG,
+ (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg("too many background workers"),
+ errhint("max_connections plus autovacuum_max_workers plus the total number of background workers cannot exceed %d",
+ MAX_BACKENDS - 1)));
+ return;
+ }
+
/* sanity check for flags */
if (worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION)
{
*** a/src/backend/storage/ipc/procsignal.c
--- b/src/backend/storage/ipc/procsignal.c
***************
*** 21,26 ****
--- 21,27 ----
#include "miscadmin.h"
#include "storage/latch.h"
#include "storage/ipc.h"
+ #include "storage/proc.h"
#include "storage/shmem.h"
#include "storage/sinval.h"
#include "tcop/tcopprot.h"
*** a/src/backend/utils/init/globals.c
--- b/src/backend/utils/init/globals.c
***************
*** 103,115 **** int work_mem = 1024;
int maintenance_work_mem = 16384;
/*
! * Primary determinants of sizes of shared-memory structures. MaxBackends is
! * MaxConnections + autovacuum_max_workers + 1 (it is computed by the GUC
! * assign hooks for those variables):
*/
int NBuffers = 1000;
- int MaxBackends = 100;
int MaxConnections = 90;
int VacuumCostPageHit = 1; /* GUC parameters for vacuum */
int VacuumCostPageMiss = 10;
--- 103,113 ----
int maintenance_work_mem = 16384;
/*
! * Primary determinants of sizes of shared-memory structures.
*/
int NBuffers = 1000;
int MaxConnections = 90;
+ int autovacuum_max_workers = 3;
int VacuumCostPageHit = 1; /* GUC parameters for vacuum */
int VacuumCostPageMiss = 10;
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 103,118 ****
#define MAX_KILOBYTES (INT_MAX / 1024)
#endif
- /*
- * Note: MAX_BACKENDS is limited to 2^23-1 because inval.c stores the
- * backend ID as a 3-byte signed integer. Even if that limitation were
- * removed, we still could not exceed INT_MAX/4 because some places compute
- * 4*MaxBackends without any overflow check. This is rechecked in
- * check_maxconnections, since MaxBackends is computed as MaxConnections
- * plus the number of bgworkers plus autovacuum_max_workers plus one (for the
- * autovacuum launcher).
- */
- #define MAX_BACKENDS 0x7fffff
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
--- 103,108 ----
***************
*** 199,207 **** static const char *show_tcp_keepalives_idle(void);
static const char *show_tcp_keepalives_interval(void);
static const char *show_tcp_keepalives_count(void);
static bool check_maxconnections(int *newval, void **extra, GucSource source);
- static void assign_maxconnections(int newval, void *extra);
static bool check_autovacuum_max_workers(int *newval, void **extra, GucSource source);
- static void assign_autovacuum_max_workers(int newval, void *extra);
static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source);
static void assign_effective_io_concurrency(int newval, void *extra);
static void assign_pgstat_temp_directory(const char *newval, void *extra);
--- 189,195 ----
***************
*** 1615,1621 **** static struct config_int ConfigureNamesInt[] =
},
&MaxConnections,
100, 1, MAX_BACKENDS,
! check_maxconnections, assign_maxconnections, NULL
},
{
--- 1603,1609 ----
},
&MaxConnections,
100, 1, MAX_BACKENDS,
! check_maxconnections, NULL, NULL
},
{
***************
*** 2290,2296 **** static struct config_int ConfigureNamesInt[] =
},
&autovacuum_max_workers,
3, 1, MAX_BACKENDS,
! check_autovacuum_max_workers, assign_autovacuum_max_workers, NULL
},
{
--- 2278,2284 ----
},
&autovacuum_max_workers,
3, 1, MAX_BACKENDS,
! check_autovacuum_max_workers, NULL, NULL
},
{
***************
*** 8630,8648 **** show_tcp_keepalives_count(void)
static bool
check_maxconnections(int *newval, void **extra, GucSource source)
{
! if (*newval + GetNumShmemAttachedBgworkers() + autovacuum_max_workers + 1 >
MAX_BACKENDS)
return false;
return true;
}
- static void
- assign_maxconnections(int newval, void *extra)
- {
- MaxBackends = newval + autovacuum_max_workers + 1 +
- GetNumShmemAttachedBgworkers();
- }
-
static bool
check_autovacuum_max_workers(int *newval, void **extra, GucSource source)
{
--- 8618,8629 ----
static bool
check_maxconnections(int *newval, void **extra, GucSource source)
{
! if (*newval + autovacuum_max_workers + 1 + GetNumShmemAttachedBgworkers() >
MAX_BACKENDS)
return false;
return true;
}
static bool
check_autovacuum_max_workers(int *newval, void **extra, GucSource source)
{
***************
*** 8652,8663 **** check_autovacuum_max_workers(int *newval, void **extra, GucSource source)
return true;
}
- static void
- assign_autovacuum_max_workers(int newval, void *extra)
- {
- MaxBackends = MaxConnections + newval + 1 + GetNumShmemAttachedBgworkers();
- }
-
static bool
check_effective_io_concurrency(int *newval, void **extra, GucSource source)
{
--- 8633,8638 ----
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
***************
*** 139,146 **** extern bool ExitOnAnyError;
extern PGDLLIMPORT char *DataDir;
extern PGDLLIMPORT int NBuffers;
- extern int MaxBackends;
- extern int MaxConnections;
extern PGDLLIMPORT int MyProcPid;
extern PGDLLIMPORT pg_time_t MyStartTime;
--- 139,144 ----
*** a/src/include/postmaster/autovacuum.h
--- b/src/include/postmaster/autovacuum.h
***************
*** 17,23 ****
/* GUC variables */
extern bool autovacuum_start_daemon;
! extern int autovacuum_max_workers;
extern int autovacuum_naptime;
extern int autovacuum_vac_thresh;
extern double autovacuum_vac_scale;
--- 17,23 ----
/* GUC variables */
extern bool autovacuum_start_daemon;
! /* autovacuum_max_workers is elsewhere */
extern int autovacuum_naptime;
extern int autovacuum_vac_thresh;
extern double autovacuum_vac_scale;
*** a/src/include/storage/proc.h
--- b/src/include/storage/proc.h
***************
*** 15,24 ****
--- 15,43 ----
#define _PROC_H_
#include "access/xlogdefs.h"
+ #include "postmaster/postmaster.h"
#include "storage/latch.h"
#include "storage/lock.h"
#include "storage/pg_sema.h"
+
+ /*
+ * GUC variables determining the maximum number of backend processes.
+ */
+ extern int MaxConnections;
+ extern int autovacuum_max_workers;
+ #define MaxBackends (MaxConnections + autovacuum_max_workers + 1 + \
+ GetNumShmemAttachedBgworkers())
+ /*
+ * Note: MAX_BACKENDS is limited to 2^23-1 because inval.c stores the
+ * backend ID as a 3-byte signed integer. Even if that limitation were
+ * removed, we still could not exceed INT_MAX/4 because some places compute
+ * 4*MaxBackends without any overflow check. This is rechecked in
+ * check_maxconnections and check_autovacuum_max_workers, and also in the
+ * background worker registration code.
+ */
+ #define MAX_BACKENDS 0x7fffff
+
/*
* Each backend advertises up to PGPROC_MAX_CACHED_SUBXIDS TransactionIds
* for non-aborted subtransactions of its current top transaction. These
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
