Add a new shmem_request_hook hook.
authorRobert Haas <rhaas@postgresql.org>
Fri, 13 May 2022 13:31:06 +0000 (09:31 -0400)
committerRobert Haas <rhaas@postgresql.org>
Fri, 13 May 2022 13:31:06 +0000 (09:31 -0400)
Currently, preloaded libraries are expected to request additional
shared memory and LWLocks in _PG_init().  However, it is not unusal
for such requests to depend on MaxBackends, which won't be
initialized at that time.  Such requests could also depend on GUCs
that other modules might change.  This introduces a new hook where
modules can safely use MaxBackends and GUCs to request additional
shared memory and LWLocks.

Furthermore, this change restricts requests for shared memory and
LWLocks to this hook.  Previously, libraries could make requests
until the size of the main shared memory segment was calculated.
Unlike before, we no longer silently ignore requests received at
invalid times.  Instead, we FATAL if someone tries to request
additional shared memory or LWLocks outside of the hook.

Nathan Bossart and Julien Rouhaud

Discussion: https://postgr.es/m/20220412210112.GA2065815%40nathanxps13
Discussion: https://postgr.es/m/Yn2jE/lmDhKtkUdr@paquier.xyz

contrib/pg_prewarm/autoprewarm.c
contrib/pg_stat_statements/pg_stat_statements.c
doc/src/sgml/xfunc.sgml
src/backend/postmaster/postmaster.c
src/backend/storage/ipc/ipci.c
src/backend/storage/lmgr/lwlock.c
src/backend/utils/init/miscinit.c
src/include/miscadmin.h
src/tools/pgindent/typedefs.list

index 45e012a63a55d48858145a620457ed33de163a24..c0c4f5d9ca7c9ee04ca5c19a095a6706ef0b441c 100644 (file)
@@ -96,6 +96,8 @@ static void apw_start_database_worker(void);
 static bool apw_init_shmem(void);
 static void apw_detach_shmem(int code, Datum arg);
 static int apw_compare_blockinfo(const void *p, const void *q);
+static void autoprewarm_shmem_request(void);
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
 
 /* Pointer to shared-memory state. */
 static AutoPrewarmSharedState *apw_state = NULL;
@@ -139,13 +141,26 @@ _PG_init(void)
 
    MarkGUCPrefixReserved("pg_prewarm");
 
-   RequestAddinShmemSpace(MAXALIGN(sizeof(AutoPrewarmSharedState)));
+   prev_shmem_request_hook = shmem_request_hook;
+   shmem_request_hook = autoprewarm_shmem_request;
 
    /* Register autoprewarm worker, if enabled. */
    if (autoprewarm)
        apw_start_leader_worker();
 }
 
+/*
+ * Requests any additional shared memory required for autoprewarm.
+ */
+static void
+autoprewarm_shmem_request(void)
+{
+   if (prev_shmem_request_hook)
+       prev_shmem_request_hook();
+
+   RequestAddinShmemSpace(MAXALIGN(sizeof(AutoPrewarmSharedState)));
+}
+
 /*
  * Main entry point for the leader autoprewarm process.  Per-database workers
  * have a separate entry point.
index 5c8b9ff94300de5e48ad9d496bd03b36a5cab901..768cedd91a75f9393ecede9a1d8640ec183cc6b2 100644 (file)
@@ -252,6 +252,7 @@ static int  exec_nested_level = 0;
 static int plan_nested_level = 0;
 
 /* Saved hook values in case of unload */
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
 static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
 static post_parse_analyze_hook_type prev_post_parse_analyze_hook = NULL;
 static planner_hook_type prev_planner_hook = NULL;
@@ -316,6 +317,7 @@ PG_FUNCTION_INFO_V1(pg_stat_statements_1_10);
 PG_FUNCTION_INFO_V1(pg_stat_statements);
 PG_FUNCTION_INFO_V1(pg_stat_statements_info);
 
+static void pgss_shmem_request(void);
 static void pgss_shmem_startup(void);
 static void pgss_shmem_shutdown(int code, Datum arg);
 static void pgss_post_parse_analyze(ParseState *pstate, Query *query,
@@ -451,17 +453,11 @@ _PG_init(void)
 
    MarkGUCPrefixReserved("pg_stat_statements");
 
-   /*
-    * Request additional shared resources.  (These are no-ops if we're not in
-    * the postmaster process.)  We'll allocate or attach to the shared
-    * resources in pgss_shmem_startup().
-    */
-   RequestAddinShmemSpace(pgss_memsize());
-   RequestNamedLWLockTranche("pg_stat_statements", 1);
-
    /*
     * Install hooks.
     */
+   prev_shmem_request_hook = shmem_request_hook;
+   shmem_request_hook = pgss_shmem_request;
    prev_shmem_startup_hook = shmem_startup_hook;
    shmem_startup_hook = pgss_shmem_startup;
    prev_post_parse_analyze_hook = post_parse_analyze_hook;
@@ -480,6 +476,20 @@ _PG_init(void)
    ProcessUtility_hook = pgss_ProcessUtility;
 }
 
+/*
+ * shmem_request hook: request additional shared resources.  We'll allocate or
+ * attach to the shared resources in pgss_shmem_startup().
+ */
+static void
+pgss_shmem_request(void)
+{
+   if (prev_shmem_request_hook)
+       prev_shmem_request_hook();
+
+   RequestAddinShmemSpace(pgss_memsize());
+   RequestNamedLWLockTranche("pg_stat_statements", 1);
+}
+
 /*
  * shmem_startup hook: allocate or attach to shared memory,
  * then load any pre-existing statistics from file.
index fbe718e3c28af9aa916ef8286b991143e0bd370b..3b0adc0704bc4d702982f217dff47c2dd5a1e59f 100644 (file)
@@ -3402,22 +3402,30 @@ CREATE FUNCTION make_array(anyelement) RETURNS anyarray
      startup.  The add-in's shared library must be preloaded by specifying
      it in
      <xref linkend="guc-shared-preload-libraries"/><indexterm><primary>shared_preload_libraries</primary></indexterm>.
+     The shared library should register a <literal>shmem_request_hook</literal>
+     in its <function>_PG_init</function> function.  This
+     <literal>shmem_request_hook</literal> can reserve LWLocks or shared memory.
      Shared memory is reserved by calling:
 <programlisting>
 void RequestAddinShmemSpace(int size)
 </programlisting>
-     from your <function>_PG_init</function> function.
+     from your <literal>shmem_request_hook</literal>.
     </para>
     <para>
      LWLocks are reserved by calling:
 <programlisting>
 void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
 </programlisting>
-     from <function>_PG_init</function>.  This will ensure that an array of
+     from your <literal>shmem_request_hook</literal>.  This will ensure that an array of
      <literal>num_lwlocks</literal> LWLocks is available under the name
      <literal>tranche_name</literal>.  Use <function>GetNamedLWLockTranche</function>
      to get a pointer to this array.
     </para>
+    <para>
+     An example of a <literal>shmem_request_hook</literal> can be found in
+     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
+     <productname>PostgreSQL</productname> source tree.
+    </para>
     <para>
      To avoid possible race-conditions, each backend should use the LWLock
      <function>AddinShmemInitLock</function> when connecting to and initializing
index bf591f048d4d38dd4fc6437dc2c751738338dd33..3b73e26956473cf872bdc700510f410d72e45749 100644 (file)
@@ -1042,6 +1042,11 @@ PostmasterMain(int argc, char *argv[])
     */
    InitializeMaxBackends();
 
+   /*
+    * Give preloaded libraries a chance to request additional shared memory.
+    */
+   process_shmem_requests();
+
    /*
     * Now that loadable modules have had their chance to request additional
     * shared memory, determine the value of any runtime-computed GUCs that
index 75e456360bef90c4cc210fc9495047c4cb2a884a..26372d95b3856c6014f24dcfe448461563ec5729 100644 (file)
@@ -55,25 +55,21 @@ int         shared_memory_type = DEFAULT_SHARED_MEMORY_TYPE;
 shmem_startup_hook_type shmem_startup_hook = NULL;
 
 static Size total_addin_request = 0;
-static bool addin_request_allowed = true;
-
 
 /*
  * RequestAddinShmemSpace
  *     Request that extra shmem space be allocated for use by
  *     a loadable module.
  *
- * This is only useful if called from the _PG_init hook of a library that
- * is loaded into the postmaster via shared_preload_libraries.  Once
- * shared memory has been allocated, calls will be ignored.  (We could
- * raise an error, but it seems better to make it a no-op, so that
- * libraries containing such calls can be reloaded if needed.)
+ * This may only be called via the shmem_request_hook of a library that is
+ * loaded into the postmaster via shared_preload_libraries.  Calls from
+ * elsewhere will fail.
  */
 void
 RequestAddinShmemSpace(Size size)
 {
-   if (IsUnderPostmaster || !addin_request_allowed)
-       return;                 /* too late */
+   if (!process_shmem_requests_in_progress)
+       elog(FATAL, "cannot request additional shared memory outside shmem_request_hook");
    total_addin_request = add_size(total_addin_request, size);
 }
 
@@ -83,9 +79,6 @@ RequestAddinShmemSpace(Size size)
  *
  * If num_semaphores is not NULL, it will be set to the number of semaphores
  * required.
- *
- * Note that this function freezes the additional shared memory request size
- * from loadable modules.
  */
 Size
 CalculateShmemSize(int *num_semaphores)
@@ -152,8 +145,7 @@ CalculateShmemSize(int *num_semaphores)
    size = add_size(size, ShmemBackendArraySize());
 #endif
 
-   /* freeze the addin request size and include it */
-   addin_request_allowed = false;
+   /* include additional requested shmem from preload libraries */
    size = add_size(size, total_addin_request);
 
    /* might as well round it off to a multiple of a typical page size */
index fef462b11026e9fbb5e8fb65c0bda16422929509..8aef909037da712481cec1d2bed2d10f5f560f50 100644 (file)
@@ -243,8 +243,6 @@ int         NamedLWLockTrancheRequests = 0;
 /* points to data in shared memory: */
 NamedLWLockTranche *NamedLWLockTrancheArray = NULL;
 
-static bool lock_named_request_allowed = true;
-
 static void InitializeLWLocks(void);
 static inline void LWLockReportWaitStart(LWLock *lock);
 static inline void LWLockReportWaitEnd(void);
@@ -458,9 +456,6 @@ LWLockShmemSize(void)
    for (i = 0; i < NamedLWLockTrancheRequests; i++)
        size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1);
 
-   /* Disallow adding any more named tranches. */
-   lock_named_request_allowed = false;
-
    return size;
 }
 
@@ -691,12 +686,9 @@ LWLockRegisterTranche(int tranche_id, const char *tranche_name)
  *     Request that extra LWLocks be allocated during postmaster
  *     startup.
  *
- * This is only useful for extensions if called from the _PG_init hook
- * of a library that is loaded into the postmaster via
- * shared_preload_libraries.  Once shared memory has been allocated, calls
- * will be ignored.  (We could raise an error, but it seems better to make
- * it a no-op, so that libraries containing such calls can be reloaded if
- * needed.)
+ * This may only be called via the shmem_request_hook of a library that is
+ * loaded into the postmaster via shared_preload_libraries.  Calls from
+ * elsewhere will fail.
  *
  * The tranche name will be user-visible as a wait event name, so try to
  * use a name that fits the style for those.
@@ -706,8 +698,8 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
 {
    NamedLWLockTrancheRequest *request;
 
-   if (IsUnderPostmaster || !lock_named_request_allowed)
-       return;                 /* too late */
+   if (!process_shmem_requests_in_progress)
+       elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook");
 
    if (NamedLWLockTrancheRequestArray == NULL)
    {
index 6ed584e114313a74177c03d6bb096075f80f89ba..ec6a61594a4b27f12896eeb05d0f49136b992499 100644 (file)
@@ -1618,6 +1618,9 @@ char     *local_preload_libraries_string = NULL;
 bool       process_shared_preload_libraries_in_progress = false;
 bool       process_shared_preload_libraries_done = false;
 
+shmem_request_hook_type shmem_request_hook = NULL;
+bool       process_shmem_requests_in_progress = false;
+
 /*
  * load the shared libraries listed in 'libraries'
  *
@@ -1701,6 +1704,18 @@ process_session_preload_libraries(void)
                   true);
 }
 
+/*
+ * process any shared memory requests from preloaded libraries
+ */
+void
+process_shmem_requests(void)
+{
+   process_shmem_requests_in_progress = true;
+   if (shmem_request_hook)
+       shmem_request_hook();
+   process_shmem_requests_in_progress = false;
+}
+
 void
 pg_bindtextdomain(const char *domain)
 {
index 53fd168d93c3d2e83c43600d1e2e3110c278e740..0af130fbc5d84251d09d0d32e1369a66cfe1af71 100644 (file)
@@ -465,6 +465,7 @@ extern void BaseInit(void);
 extern PGDLLIMPORT bool IgnoreSystemIndexes;
 extern PGDLLIMPORT bool process_shared_preload_libraries_in_progress;
 extern PGDLLIMPORT bool process_shared_preload_libraries_done;
+extern PGDLLIMPORT bool process_shmem_requests_in_progress;
 extern PGDLLIMPORT char *session_preload_libraries_string;
 extern PGDLLIMPORT char *shared_preload_libraries_string;
 extern PGDLLIMPORT char *local_preload_libraries_string;
@@ -478,9 +479,13 @@ extern bool RecheckDataDirLockFile(void);
 extern void ValidatePgVersion(const char *path);
 extern void process_shared_preload_libraries(void);
 extern void process_session_preload_libraries(void);
+extern void process_shmem_requests(void);
 extern void pg_bindtextdomain(const char *domain);
 extern bool has_rolreplication(Oid roleid);
 
+typedef void (*shmem_request_hook_type) (void);
+extern PGDLLIMPORT shmem_request_hook_type shmem_request_hook;
+
 /* in executor/nodeHash.c */
 extern size_t get_hash_memory_limit(void);
 
index dd1214977a87df410f41549b4162c44a14f44304..4fb746930aa099e5b926307b3a3c792ad8be90ba 100644 (file)
@@ -3651,6 +3651,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