public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bossart, Nathan <[email protected]>
To: Fujii Masao <[email protected]>
To: [email protected] <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Greg Sabino Mullane <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: make MaxBackends available in _PG_init
Date: Fri, 7 Jan 2022 20:20:02 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <4f20d57b2aeb447b8eb1495319940c5f@G08CNEXMBPEKD06.g08.fujitsu.local>
<CALj2ACUHruScDf2dvnVso03bhSdr7MVvLie5C8ND1dR+hJrFEg@mail.gmail.com>
<[email protected]>
<CA+TgmoYKBMdizs7-NBrhLHJXCv4dAAmBKHvER0PDmV+rBCy7KA@mail.gmail.com>
<[email protected]>
<CA+TgmoaaF46+A_Y-DTxDJpPGcqKxJU=ZPzFisqqouRL_WxYBSQ@mail.gmail.com>
<[email protected]>
<CAKAnmm+o5-ROvnghC-79=1Jr4bqJxmyu98Qax=KSukdL6JZm0g@mail.gmail.com>
<[email protected]>
<CAKAnmmJsHnNc65aLQErabTdcF=B+bMfSnLNUezC7nnsMb8bCDg@mail.gmail.com>
<[email protected]>
<CAKAnmmJ_3HQZ7PvpiJoJKYfffp8GOUXE3BVHsD9EOjDrcrU1FA@mail.gmail.com>
<OSBPR01MB42140A01E3441C576539AF38F2FC9@OSBPR01MB4214.jpnprd01.prod.outlook.com>
<[email protected]>
<[email protected]>
<[email protected]>
On 1/7/22, 10:12 AM, "Bossart, Nathan" <[email protected]> wrote:
> While that approach would provide a way to safely retrieve the value,
> I think it would do little to prevent the issue in practice. If the
> size of the patch is a concern, we could also convert MaxBackends into
> a macro for calling GetMaxBackends(). This could also be a nice way
> to avoid breaking extensions that are using it correctly while
> triggering ERRORs for extensions that are using it incorrectly. I've
> attached a new version of the patch that does it this way.
v5 didn't work with EXEC_BACKEND. Here is a new revision.
Nathan
Attachments:
[application/octet-stream] v6-0001-Disallow-external-access-to-MaxBackends.patch (7.2K, ../[email protected]/2-v6-0001-Disallow-external-access-to-MaxBackends.patch)
download | inline diff:
From 615dfbd3352a235aa615098a13e3de33edb60c5a Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Fri, 7 Jan 2022 19:50:57 +0000
Subject: [PATCH v6 1/1] Disallow external access to MaxBackends.
Presently, MaxBackends is externally visible, but it may still be
uninitialized in places where it would be convenient to use (e.g.,
_PG_init()). This change renames MaxBackends to
MaxBackendsInternal, makes it static to postinit.c to disallow
direct access, and introduces a MaxBackends macro for calling
GetMaxBackends().
Separately, adjust the comments about needing to register
background workers before initializing MaxBackends. Since
6bc8ef0b, InitializeMaxBackends() has used max_worker_processes
instead of tallying up the number of registered background workers,
so background worker registration is no longer a prerequisite. The
ordering of this logic is still useful for allowing libraries to
adjust GUCs, so the comments have been updated to mention that use-
case.
---
src/backend/postmaster/postmaster.c | 16 ++++++-------
src/backend/utils/init/globals.c | 4 ----
src/backend/utils/init/postinit.c | 47 ++++++++++++++++++++++++++++++-------
src/include/miscadmin.h | 5 +++-
4 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 328ecafa8c..1609e24114 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -525,7 +525,7 @@ typedef struct
bool IsBinaryUpgrade;
bool query_id_enabled;
int max_safe_fds;
- int MaxBackends;
+ int max_backends;
#ifdef WIN32
HANDLE PostmasterHandle;
HANDLE initial_signal_pipe;
@@ -1014,10 +1014,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 it before
+ * any modules had chance to take the background worker slots.
*/
ApplyLauncherRegister();
@@ -1038,8 +1036,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();
@@ -6262,7 +6260,7 @@ save_backend_variables(BackendParameters *param, Port *port,
param->query_id_enabled = query_id_enabled;
param->max_safe_fds = max_safe_fds;
- param->MaxBackends = MaxBackends;
+ param->max_backends = MaxBackends;
#ifdef WIN32
param->PostmasterHandle = PostmasterHandle;
@@ -6496,7 +6494,7 @@ restore_backend_variables(BackendParameters *param, Port *port)
query_id_enabled = param->query_id_enabled;
max_safe_fds = param->max_safe_fds;
- MaxBackends = param->MaxBackends;
+ SetMaxBackends(param->max_backends);
#ifdef WIN32
PostmasterHandle = param->PostmasterHandle;
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 381d9e548d..47884bbe85 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -128,15 +128,11 @@ int max_parallel_maintenance_workers = 2;
/*
* Primary determinants of sizes of shared-memory structures.
- *
- * MaxBackends is computed by PostmasterMain after modules have had a chance to
- * register background workers.
*/
int NBuffers = 1000;
int MaxConnections = 90;
int max_worker_processes = 8;
int max_parallel_workers = 8;
-int MaxBackends = 0;
int VacuumCostPageHit = 1; /* GUC parameters for vacuum */
int VacuumCostPageMiss = 2;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 7292e51f7d..aeeb709a67 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -63,6 +63,9 @@
#include "utils/syscache.h"
#include "utils/timeout.h"
+static bool MaxBackendsInitialized = false;
+static int MaxBackendsInternal = 0;
+
static HeapTuple GetDatabaseTuple(const char *dbname);
static HeapTuple GetDatabaseTupleByOid(Oid dboid);
static void PerformAuthentication(Port *port);
@@ -476,9 +479,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
@@ -488,15 +490,42 @@ pg_split_opts(char **argv, int *argcp, const char *optstr)
void
InitializeMaxBackends(void)
{
- Assert(MaxBackends == 0);
-
/* the extra unit accounts for the autovacuum launcher */
- MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- max_worker_processes + max_wal_senders;
+ SetMaxBackends(MaxConnections + autovacuum_max_workers + 1 +
+ max_worker_processes + max_wal_senders);
+}
- /* internal error because the values were all checked previously */
- if (MaxBackends > MAX_BACKENDS)
+/*
+ * Retrieve the value for MaxBackends.
+ *
+ * If the value has not yet been initialized, this function will ERROR.
+ */
+int
+GetMaxBackends(void)
+{
+ if (!MaxBackendsInitialized)
+ elog(ERROR, "MaxBackends not yet initialized");
+
+ return MaxBackendsInternal;
+}
+
+/*
+ * Set the value for MaxBackends.
+ *
+ * If the value is already initialized or the value provided is not valid, this
+ * function will ERROR.
+ */
+void
+SetMaxBackends(int max_backends)
+{
+ if (MaxBackendsInitialized)
+ elog(ERROR, "MaxBackends already initialized");
+
+ if (max_backends > MAX_BACKENDS)
elog(ERROR, "too many backends configured");
+
+ MaxBackendsInternal = max_backends;
+ MaxBackendsInitialized = true;
}
/*
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 90a3016065..4860a9540a 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -172,7 +172,6 @@ extern PGDLLIMPORT char *DataDir;
extern PGDLLIMPORT int data_directory_mode;
extern PGDLLIMPORT int NBuffers;
-extern PGDLLIMPORT int MaxBackends;
extern PGDLLIMPORT int MaxConnections;
extern PGDLLIMPORT int max_worker_processes;
extern PGDLLIMPORT int max_parallel_workers;
@@ -457,10 +456,14 @@ extern AuxProcType MyAuxProcType;
/* in utils/init/postinit.c */
extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
extern void InitializeMaxBackends(void);
+extern int GetMaxBackends(void);
+extern void SetMaxBackends(int max_backends);
extern void InitPostgres(const char *in_dbname, Oid dboid, const char *username,
Oid useroid, char *out_dbname, bool override_allow_connections);
extern void BaseInit(void);
+#define MaxBackends (GetMaxBackends())
+
/* in utils/init/miscinit.c */
extern bool IgnoreSystemIndexes;
extern PGDLLIMPORT bool process_shared_preload_libraries_in_progress;
--
2.16.6
view thread (5+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: make MaxBackends available in _PG_init
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox