From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: wangsh.fnst@fujitsu.com <wangsh.fnst@fujitsu.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Greg Sabino Mullane <htamfids@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Subject: Re: make MaxBackends available in _PG_init
Date: Tue, 12 Apr 2022 15:26:16 -0700
Message-ID: <20220412222616.GA2113503@nathanxps13> (raw)
In-Reply-To: <995617.1649799996@sss.pgh.pa.us>
References: <20220411211435.GA2057981@nathanxps13>
<20220412174427.sngj2uexzsttazs2@alap3.anarazel.de>
<20220412180306.GA2063516@nathanxps13>
<CA+TgmoY4og1VADa78VySwsTjtTQbtXbnRPSq9x=e6ogS_wWfaw@mail.gmail.com>
<908945.1649791317@sss.pgh.pa.us>
<CA+TgmoZM7=VoAey4rf+sZEKFpGDWhqEqk_MHoBPXkYbAONGcyQ@mail.gmail.com>
<20220412195908.GA2064996@nathanxps13>
<918045.1649797122@sss.pgh.pa.us>
<20220412210745.GB2065815@nathanxps13>
<995617.1649799996@sss.pgh.pa.us>
On Tue, Apr 12, 2022 at 05:46:36PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> If we allow changing GUCs in _PG_init() and provide another hook where
>> MaxBackends will be initialized, do we need to introduce another GUC flag,
>> or can we get away with just blocking all GUC changes when the new hook is
>> called? I'm slightly hesitant to add a GUC flag that will need to manually
>> maintained. Wouldn't it be easily forgotten?
>
> On the whole I think Robert's got the right idea: we do not really
> need an enforcement mechanism at all. People who are writing
> extensions that do this sort of thing will learn how to do it right.
> (It's not like there's not a thousand other things they have to get
> right.)
Okay. So maybe we only need the attached patches. 0001 is just 5ecd018
un-reverted, and 0002 is Julien's patch from a few weeks ago with some
minor edits.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.comAttachments:
[text/x-diff] v3-0001-Fix-comments-about-bgworker-registration-before-M.patch (3.0K, ../20220412222616.GA2113503@nathanxps13/2-v3-0001-Fix-comments-about-bgworker-registration-before-M.patch)
download | inline diff:
From 4c5ebca537ffdfbf61079a82b18ce7bc97222c69 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Tue, 12 Apr 2022 14:57:00 -0700
Subject: [PATCH v3 1/2] Fix comments about bgworker registration before
MaxBackends initialization
Since 6bc8ef0b, InitializeMaxBackends() has used max_worker_processes
instead of adapting MaxBackends to the number of background workers
registered by modules loaded in shared_preload_libraries (at this time,
bgworkers were only static, but gained dynamic capabilities as a matter
of supporting parallel queries meaning that a control cap was
necessary).
Some comments referred to the past registration logic, making them
confusing and incorrect, so fix these.
Some of the out-of-core modules that could be loaded in this path
sometimes like to manipulate dynamically some of the resource-related
GUCs for their own needs, this commit adds a note about that.
Author: Nathan Bossart
Discussion: https://postgr.es/m/20220127181815.GA551692@nathanxps13
---
src/backend/postmaster/postmaster.c | 10 ++++------
src/backend/utils/init/postinit.c | 5 ++---
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.cindex 3dcaf8a4a5..d57fefa9a8 100644--- a/src/backend/postmaster/postmaster.c+++ b/src/backend/postmaster/postmaster.c@@ -1005,10 +1005,8 @@ PostmasterMain(int argc, char *argv[])
LocalProcessControlFile(false);
/*
- * Register the apply launcher. Since it registers a background worker,- * it needs to be called before InitializeMaxBackends(), and it's probably- * a good idea to call it before any modules had chance to take the- * background worker slots.+ * Register the apply launcher. It's probably a good idea to call this+ * before any modules had a chance to take the background worker slots.
*/
ApplyLauncherRegister();
@@ -1029,8 +1027,8 @@ PostmasterMain(int argc, char *argv[])
#endif
/*
- * Now that loadable modules have had their chance to register background- * workers, calculate MaxBackends.+ * Now that loadable modules have had their chance to alter any GUCs,+ * calculate MaxBackends.
*/
InitializeMaxBackends();
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.cindex 9139fe895c..a28612b375 100644--- a/src/backend/utils/init/postinit.c+++ b/src/backend/utils/init/postinit.c@@ -538,9 +538,8 @@ pg_split_opts(char **argv, int *argcp, const char *optstr)
/*
* Initialize MaxBackends value from config options.
*
- * This must be called after modules have had the chance to register background- * workers in shared_preload_libraries, and before shared memory size is- * determined.+ * This must be called after modules have had the chance to alter GUCs in+ * shared_preload_libraries and before shared memory size is determined.
*
* Note that in EXEC_BACKEND environment, the value is passed down from
* postmaster to subprocesses via BackendParameters in SubPostmasterMain; only
--
2.25.1
[text/x-diff] v3-0002-Add-a-new-shmem_request_hook-hook.patch (2.9K, ../20220412222616.GA2113503@nathanxps13/3-v3-0002-Add-a-new-shmem_request_hook-hook.patch)
download | inline diff:
From 862a5b9a8a27995157a8b71b8f3ce09bf6ed17c5 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Fri, 25 Mar 2022 11:05:24 +0800
Subject: [PATCH v3 2/2] Add a new shmem_request_hook hook.
Currently, preloaded libraries are expected to request additional
shared memory in _PG_init(). However, it is not unusal for such
requests to depend on MaxBackends, which won't be initialized at
that time. This introduces a new hook where modules can use
MaxBackends to request additional shared memory.
Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220412210112.GA2065815%40nathanxps13
---
src/backend/storage/ipc/ipci.c | 15 +++++++++++++++
src/include/storage/ipc.h | 2 ++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 18 insertions(+)
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.cindex 75e456360b..fdd72175d5 100644--- a/src/backend/storage/ipc/ipci.c+++ b/src/backend/storage/ipc/ipci.c@@ -52,6 +52,7 @@
/* GUCs */
int shared_memory_type = DEFAULT_SHARED_MEMORY_TYPE;
+shmem_request_hook_type shmem_request_hook = NULL;
shmem_startup_hook_type shmem_startup_hook = NULL;
static Size total_addin_request = 0;
@@ -152,6 +153,20 @@ CalculateShmemSize(int *num_semaphores)
size = add_size(size, ShmemBackendArraySize());
#endif
+ /*+ * Final chance for modules to request additional shared memory.+ *+ * Ordinarily, modules call RequestAddinShmemSpace() in _PG_init(), but it+ * is not unusal for such requests to depend on MaxBackends, which won't be+ * initialized at that time. This hook provides an opportunity for modules+ * to use MaxBackends when requesting shared memory.+ */+ if (addin_request_allowed && shmem_request_hook)+ {+ Assert(MaxBackends > 0);+ shmem_request_hook();+ }+
/* freeze the addin request size and include it */
addin_request_allowed = false;
size = add_size(size, total_addin_request);
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.hindex fade4dbe63..5f2c6683db 100644--- a/src/include/storage/ipc.h+++ b/src/include/storage/ipc.h@@ -19,6 +19,7 @@
#define IPC_H
typedef void (*pg_on_exit_callback) (int code, Datum arg);
+typedef void (*shmem_request_hook_type) (void);
typedef void (*shmem_startup_hook_type) (void);
/*----------
@@ -75,6 +76,7 @@ extern void on_exit_reset(void);
extern void check_on_shmem_exit_lists_are_empty(void);
/* ipci.c */
+extern PGDLLIMPORT shmem_request_hook_type shmem_request_hook;
extern PGDLLIMPORT shmem_startup_hook_type shmem_startup_hook;
extern Size CalculateShmemSize(int *num_semaphores);
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.listindex 87ee7bf866..71a97654e0 100644--- a/src/tools/pgindent/typedefs.list+++ b/src/tools/pgindent/typedefs.list@@ -3549,6 +3549,7 @@ shm_mq_result
shm_toc
shm_toc_entry
shm_toc_estimator
+shmem_request_hook_type
shmem_startup_hook_type
sig_atomic_t
sigjmp_buf
--
2.25.1
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, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, andres@anarazel.de, rjuju123@gmail.com, michael@paquier.xyz, bossartn@amazon.com, masao.fujii@oss.nttdata.com, wangsh.fnst@fujitsu.com, bharath.rupireddyforpostgres@gmail.com, htamfids@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: make MaxBackends available in _PG_init
In-Reply-To: <20220412222616.GA2113503@nathanxps13>
* 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