agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedremove check hooks for GUCs that contribute to MaxBackends
5+ messages / 2 participants
[nested] [flat]
* remove check hooks for GUCs that contribute to MaxBackends
@ 2024-06-19 19:04 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 5+ messages in thread
From: Nathan Bossart @ 2024-06-19 19:04 UTC (permalink / raw)
To: pgsql-hackers
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 <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)
Attachments:
[text/plain] v1-0001-remove-check-hooks-for-GUCs-that-contribute-to-Ma.patch (4.9K, ../../ZnMr2k-Nk5vj7T7H@nathan/2-v1-0001-remove-check-hooks-for-GUCs-that-contribute-to-Ma.patch)
download | inline diff:
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)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: remove check hooks for GUCs that contribute to MaxBackends
@ 2024-06-19 19:09 Tom Lane <tgl@sss.pgh.pa.us>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 5+ messages in thread
From: Tom Lane @ 2024-06-19 19:09 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: remove check hooks for GUCs that contribute to MaxBackends
@ 2024-06-19 19:14 Tom Lane <tgl@sss.pgh.pa.us>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 1 reply; 5+ messages in thread
From: Tom Lane @ 2024-06-19 19:14 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: remove check hooks for GUCs that contribute to MaxBackends
@ 2024-06-19 19:24 Nathan Bossart <nathandbossart@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 5+ messages in thread
From: Nathan Bossart @ 2024-06-19 19:24 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: remove check hooks for GUCs that contribute to MaxBackends
@ 2024-07-05 19:50 Nathan Bossart <nathandbossart@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 5+ messages in thread
From: Nathan Bossart @ 2024-07-05 19:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
Committed.
--
nathan
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-07-05 19:50 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-06-19 19:04 remove check hooks for GUCs that contribute to MaxBackends Nathan Bossart <nathandbossart@gmail.com>
2024-06-19 19:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-06-19 19:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-06-19 19:24 ` Nathan Bossart <nathandbossart@gmail.com>
2024-07-05 19:50 ` Nathan Bossart <nathandbossart@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox