From 615dfbd3352a235aa615098a13e3de33edb60c5a Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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