pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: pgsql-hackers@postgresql.org
Subject: remove check hooks for GUCs that contribute to MaxBackends
Date: Wed, 19 Jun 2024 14:04:58 -0500
Message-ID: <ZnMr2k-Nk5vj7T7H@nathan> (raw)

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

Attachments:

v1-0001-remove-check-hooks-for-GUCs-that-contribute-to-Ma.patch (text/plain, 4.9K) download 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)

view thread (5+ messages)  latest in thread

Message-ID: <ZnMr2k-Nk5vj7T7H@nathan>
Permalink:  ../ZnMr2k-Nk5vj7T7H@nathan/
Also on:    postgresql.org/message-id/ZnMr2k-Nk5vj7T7H@nathan

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com
  Subject: Re: remove check hooks for GUCs that contribute to MaxBackends
  In-Reply-To: <ZnMr2k-Nk5vj7T7H@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox