While working on an idea from another thread [0], I noticed that each of
max_connections, max_worker_process, max_autovacuum_workers, and
max_wal_senders have a check hook that verifies the sum of those GUCs does
not exceed a certain value.  Then, in InitializeMaxBackends(), we do the
same check once more.  Not only do the check hooks seem redundant, but I
think they might sometimes be inaccurate since some values might not yet be
initialized.  Furthermore, the error message is not exactly the most
descriptive:

        $ pg_ctl -D . start -o "-c max_connections=262100 -c 
max_wal_senders=10000"

        FATAL:  invalid value for parameter "max_wal_senders": 10000

The attached patch removes these hooks and enhances the error message to
look like this:

        FATAL:  too many backends configured
        DETAIL:  "max_connections" (262100) plus "autovacuum_max_workers" (3) 
plus "max_worker_processes" (8) plus "max_wal_senders" (10000) must be less 
than 262142.

The downside of this change is that server startup progresses a little
further before it fails, but that might not be too concerning given this
_should_ be a relatively rare occurrence.

Thoughts?

[0] https://postgr.es/m/20240618213331.ef2spg3nasksisbi%40awork3.anarazel.de

-- 
nathan
>From 2ab34581879d2122f03be0e3f9d0d15edb501a7c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Wed, 19 Jun 2024 13:39:18 -0500
Subject: [PATCH v1 1/1] remove check hooks for GUCs that contribute to
 MaxBackends

---
 src/backend/utils/init/postinit.c   | 57 ++++-------------------------
 src/backend/utils/misc/guc_tables.c |  8 ++--
 src/include/utils/guc_hooks.h       |  6 ---
 3 files changed, 11 insertions(+), 60 deletions(-)

diff --git a/src/backend/utils/init/postinit.c 
b/src/backend/utils/init/postinit.c
index 0805398e24..8a629982c4 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -580,57 +580,14 @@ InitializeMaxBackends(void)
        MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
                max_worker_processes + max_wal_senders;
 
-       /* internal error because the values were all checked previously */
        if (MaxBackends > MAX_BACKENDS)
-               elog(ERROR, "too many backends configured");
-}
-
-/*
- * GUC check_hook for max_connections
- */
-bool
-check_max_connections(int *newval, void **extra, GucSource source)
-{
-       if (*newval + autovacuum_max_workers + 1 +
-               max_worker_processes + max_wal_senders > MAX_BACKENDS)
-               return false;
-       return true;
-}
-
-/*
- * GUC check_hook for autovacuum_max_workers
- */
-bool
-check_autovacuum_max_workers(int *newval, void **extra, GucSource source)
-{
-       if (MaxConnections + *newval + 1 +
-               max_worker_processes + max_wal_senders > MAX_BACKENDS)
-               return false;
-       return true;
-}
-
-/*
- * GUC check_hook for max_worker_processes
- */
-bool
-check_max_worker_processes(int *newval, void **extra, GucSource source)
-{
-       if (MaxConnections + autovacuum_max_workers + 1 +
-               *newval + max_wal_senders > MAX_BACKENDS)
-               return false;
-       return true;
-}
-
-/*
- * GUC check_hook for max_wal_senders
- */
-bool
-check_max_wal_senders(int *newval, void **extra, GucSource source)
-{
-       if (MaxConnections + autovacuum_max_workers + 1 +
-               max_worker_processes + *newval > MAX_BACKENDS)
-               return false;
-       return true;
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("too many backends configured"),
+                                errdetail("\"max_connections\" (%d) plus 
\"autovacuum_max_workers\" (%d) plus \"max_worker_processes\" (%d) plus 
\"max_wal_senders\" (%d) must be less than %d.",
+                                                  MaxConnections, 
autovacuum_max_workers,
+                                                  max_worker_processes, 
max_wal_senders,
+                                                  MAX_BACKENDS - 1)));
 }
 
 /*
diff --git a/src/backend/utils/misc/guc_tables.c 
b/src/backend/utils/misc/guc_tables.c
index 46c258be28..07b575894d 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2207,7 +2207,7 @@ struct config_int ConfigureNamesInt[] =
                },
                &MaxConnections,
                100, 1, MAX_BACKENDS,
-               check_max_connections, NULL, NULL
+               NULL, NULL, NULL
        },
 
        {
@@ -2923,7 +2923,7 @@ struct config_int ConfigureNamesInt[] =
                },
                &max_wal_senders,
                10, 0, MAX_BACKENDS,
-               check_max_wal_senders, NULL, NULL
+               NULL, NULL, NULL
        },
 
        {
@@ -3153,7 +3153,7 @@ struct config_int ConfigureNamesInt[] =
                },
                &max_worker_processes,
                8, 0, MAX_BACKENDS,
-               check_max_worker_processes, NULL, NULL
+               NULL, NULL, NULL
        },
 
        {
@@ -3387,7 +3387,7 @@ struct config_int ConfigureNamesInt[] =
                },
                &autovacuum_max_workers,
                3, 1, MAX_BACKENDS,
-               check_autovacuum_max_workers, NULL, NULL
+               NULL, NULL, NULL
        },
 
        {
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index d64dc5fcdb..6304f0679b 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -29,8 +29,6 @@ extern bool check_application_name(char **newval, void 
**extra,
                                                                   GucSource 
source);
 extern void assign_application_name(const char *newval, void *extra);
 extern const char *show_archive_command(void);
-extern bool check_autovacuum_max_workers(int *newval, void **extra,
-                                                                               
 GucSource source);
 extern bool check_autovacuum_work_mem(int *newval, void **extra,
                                                                          
GucSource source);
 extern bool check_vacuum_buffer_usage_limit(int *newval, void **extra,
@@ -84,13 +82,9 @@ extern const char *show_log_timezone(void);
 extern bool check_maintenance_io_concurrency(int *newval, void **extra,
                                                                                
         GucSource source);
 extern void assign_maintenance_io_concurrency(int newval, void *extra);
-extern bool check_max_connections(int *newval, void **extra, GucSource source);
-extern bool check_max_wal_senders(int *newval, void **extra, GucSource source);
 extern bool check_max_slot_wal_keep_size(int *newval, void **extra,
                                                                                
 GucSource source);
 extern void assign_max_wal_size(int newval, void *extra);
-extern bool check_max_worker_processes(int *newval, void **extra,
-                                                                          
GucSource source);
 extern bool check_max_stack_depth(int *newval, void **extra, GucSource source);
 extern void assign_max_stack_depth(int newval, void *extra);
 extern bool check_multixact_member_buffers(int *newval, void **extra,
-- 
2.39.3 (Apple Git-146)

Reply via email to