remove check hooks for GUCs that contribute to MaxBackends
While working on an idea from another thread [0]/messages/by-id/20240618213331.ef2spg3nasksisbi@awork3.anarazel.de, 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]: /messages/by-id/20240618213331.ef2spg3nasksisbi@awork3.anarazel.de
--
nathan
Attachments:
v1-0001-remove-check-hooks-for-GUCs-that-contribute-to-Ma.patchtext/plain; charset=us-asciiDownload
From 2ab34581879d2122f03be0e3f9d0d15edb501a7c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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)
Nathan Bossart <nathandbossart@gmail.com> writes:
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.
Yeah, these per-variable checks are inherently bogus. If we can get
of them and make the net user experience actually better, that's a
win-win.
It seems easier to do for these because they can't change after server
start, so there can be one well-defined time to apply the consistency
check. IIRC, we have some similar issues in other hooks for variables
that aren't PGC_POSTMASTER, so it's harder to see how we might get rid
of their cross-checks. That doesn't make them less bogus though.
regards, tom lane
Nathan Bossart <nathandbossart@gmail.com> writes:
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.
BTW, I suggest writing it as "too many server processes configured",
or perhaps "too many server processes required". "Backend" is too
much of an insider term.
regards, tom lane
On Wed, Jun 19, 2024 at 03:14:16PM -0400, Tom Lane wrote:
Nathan Bossart <nathandbossart@gmail.com> writes:
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.BTW, I suggest writing it as "too many server processes configured",
or perhaps "too many server processes required". "Backend" is too
much of an insider term.
Will do, thanks for reviewing.
--
nathan