Fix background workers for EXEC_BACKEND

Commit da07a1e8 was broken for EXEC_BACKEND because I failed to realize
that the MaxBackends recomputation needed to be duplicated by
subprocesses in SubPostmasterMain.  However, instead of having the value
be recomputed at all, it's better to assign the correct value at
postmaster initialization time, and have it be propagated to exec'ed
backends via BackendParameters.

MaxBackends stays as zero until after modules in
shared_preload_libraries have had a chance to register bgworkers, since
the value is going to be untrustworthy till that's finished.

Heikki Linnakangas and Álvaro Herrera

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 73bacd89bcc90719e781429d539ac29e65586cb3..fa5aeed31db3176846bfae261d0f4096f55b5141 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -499,6 +499,7 @@ typedef struct
        bool            redirection_done;
        bool            IsBinaryUpgrade;
        int                     max_safe_fds;
+       int                     MaxBackends;
 #ifdef WIN32
        HANDLE          PostmasterHandle;
        HANDLE          initial_signal_pipe;
@@ -897,15 +898,14 @@ 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.
+        * Now that loadable modules have had their chance to register background
+        * workers, calculate MaxBackends.  Add one for the autovacuum launcher.
         */
-       if (GetNumShmemAttachedBgworkers() > 0)
-               SetConfigOption("max_connections",
-                                               GetConfigOption("max_connections", false, false),
-                                               PGC_POSTMASTER, PGC_S_OVERRIDE);
+       MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
+               GetNumShmemAttachedBgworkers();
+       /* internal error because the values were all checked previously */
+       if (MaxBackends > MAX_BACKENDS)
+               elog(ERROR, "too many backends configured");
 
        /*
         * Establish input sockets.
@@ -5152,6 +5152,8 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
 {
        RegisteredBgWorker *rw;
        int                     namelen = strlen(worker->bgw_name);
+       static int      maxworkers;
+       static int      numworkers = 0;
 
 #ifdef EXEC_BACKEND
 
@@ -5162,6 +5164,11 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
        static int      BackgroundWorkerCookie = 1;
 #endif
 
+       /* initialize upper limit on first call */
+       if (numworkers == 0)
+               maxworkers = MAX_BACKENDS -
+                       (MaxConnections + autovacuum_max_workers + 1);
+
        if (!IsUnderPostmaster)
                ereport(LOG,
                        (errmsg("registering background worker: %s", worker->bgw_name)));
@@ -5214,6 +5221,23 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
                return;
        }
 
+       /*
+        * Enforce maximum number of workers.  Note this is overly restrictive:
+        * we could allow more non-shmem-connected workers, because these don't
+        * count towards the MAX_BACKENDS limit elsewhere.  This doesn't really
+        * matter for practical purposes; several million processes would need to
+        * run on a single server.
+        */
+       if (++numworkers > maxworkers)
+       {
+               ereport(LOG,
+                               (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+                                errmsg("too many background workers"),
+                                errdetail("Up to %d background workers can be registered with the current settings.",
+                                                  maxworkers)));
+               return;
+       }
+
        /*
         * Copy the registration data into the registered workers list.
         */
@@ -5836,6 +5860,8 @@ save_backend_variables(BackendParameters *param, Port *port,
        param->IsBinaryUpgrade = IsBinaryUpgrade;
        param->max_safe_fds = max_safe_fds;
 
+       param->MaxBackends = MaxBackends;
+
 #ifdef WIN32
        param->PostmasterHandle = PostmasterHandle;
        if (!write_duplicated_handle(&param->initial_signal_pipe,
@@ -6061,6 +6087,8 @@ restore_backend_variables(BackendParameters *param, Port *port)
        IsBinaryUpgrade = param->IsBinaryUpgrade;
        max_safe_fds = param->max_safe_fds;
 
+       MaxBackends = param->MaxBackends;
+
 #ifdef WIN32
        PostmasterHandle = param->PostmasterHandle;
        pgwin32_initial_signal_pipe = param->initial_signal_pipe;

diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 00288530c07ea9b0f4e377d6a1983a3d1e0a6484..f1f8b177f35af3d5d42c73a60be6ce6c85a2f845 100644 (file)
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -103,13 +103,14 @@ 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):
+ * Primary determinants of sizes of shared-memory structures.
+ *
+ * MaxBackends is computed by PostmasterMain after modules have had a chance to
+ * register background workers.
  */
 int                    NBuffers = 1000;
-int                    MaxBackends = 100;
 int                    MaxConnections = 90;
+int                    MaxBackends = 0;
 
 int                    VacuumCostPageHit = 1;          /* GUC parameters for vacuum */
 int                    VacuumCostPageMiss = 10;

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d91924cc23459386c1742e6a53b96be3884c268e..ac5e4f3e48d308de9fa5c80aeb081c7bb699c7c2 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -103,17 +103,6 @@
 #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)
 
@@ -199,9 +188,7 @@ 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);
@@ -1615,7 +1602,7 @@ static struct config_int ConfigureNamesInt[] =
                },
                &MaxConnections,
                100, 1, MAX_BACKENDS,
-               check_maxconnections, assign_maxconnections, NULL
+               check_maxconnections, NULL, NULL
        },
 
        {
@@ -2290,7 +2277,7 @@ static struct config_int ConfigureNamesInt[] =
                },
                &autovacuum_max_workers,
                3, 1, MAX_BACKENDS,
-               check_autovacuum_max_workers, assign_autovacuum_max_workers, NULL
+               check_autovacuum_max_workers, NULL, NULL
        },
 
        {
@@ -8636,13 +8623,6 @@ check_maxconnections(int *newval, void **extra, GucSource source)
        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)
 {
@@ -8652,12 +8632,6 @@ 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)
 {

diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index eaca868b00822e0826f8967bf41d8256cec282e4..8b2d5b913e39830c78f2faf411fe831e894e6abd 100644 (file)
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -61,4 +61,13 @@ extern Size ShmemBackendArraySize(void);
 extern void ShmemBackendArrayAllocation(void);
 #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 the relevant
+ * GUC check hooks and in RegisterBackgroundWorker().
+ */
+#define MAX_BACKENDS   0x7fffff
+
 #endif   /* _POSTMASTER_H */