Subject: Re: make MaxBackends available in _PG_init

From: Julien Rouhaud <[email-masked]>
Date: Thu, 14 Apr 2022 13:50:24 +0800

On Wed, Apr 13, 2022 at 11:30:40AM -0700, Nathan Bossart wrote:
> On Wed, Apr 13, 2022 at 12:05:08PM -0400, Tom Lane wrote:
> > Robert Haas <[email-masked]> writes:
> >> It may be too much to hope that we're going to completely get rid of
> >> process_shared_preload_libraries_in_progress tests.
> > 
> > Perhaps, but this is definitely an area that could stand to have some
> > serious design thought put into it.  You're quite right that it's
> > largely a mess today, but I think proposals like "forbid setting GUCs
> > during _PG_init" would add to the mess rather than clean things up.
> 
> +1.  The simplest design might be to just make a separate preload hook.
> _PG_init() would install hooks, and then this preload hook would do
> anything that needs to happen when the library is loaded via s_p_l.
> However, I think we still want a new shmem request hook where MaxBackends
> is initialized.

Yeah this one seems needed no matter what.

> If we do move forward with the shmem request hook, do we want to disallow
> shmem requests anywhere else, or should we just leave it up to extension
> authors to do the right thing?  Many shmem requests in _PG_init() are
> probably okay unless they depend on MaxBackends or another GUC that someone
> might change.  Given that, I think I currently prefer the latter (option B
> from upthread).

I'd be in favor of a hard break.  There are already multiple extensions that
relies on non final value of GUCs to size their shmem request.  And as an
extension author it can be hard to realize that, as those extensions work just
fine until someone wants to try it with some other extension that changes some
GUC.  Forcing shmem request in a new hook will make sure that it's *always*
correct, and that only requires very minimal work on the extension side.

---

From: Julien Rouhaud <[email-masked]>
Date: Tue, 19 Apr 2022 17:49:13 +0800

Hi,

On Mon, Apr 18, 2022 at 08:17:04PM -0400, Tom Lane wrote:
> Nathan Bossart <[email-masked]> writes:
> > I'm looking for a clean way to ERROR if someone attempts to call
> > RequestAddinShmemSpace() or RequestNamedLWLockTranche() outside of the
> > hook.  Currently, we are using static variables in ipci.c and lwlock.c to
> > silently ignore invalid requests.  I could add a new 'extern bool' called
> > 'process_shmem_requests_in_progress', but extensions could easily hack
> > around that to allow requests in _PG_init().  Maybe I am overthinking all
> > this and that is good enough.
> 
> If they do that and it breaks something, that's their fault not ours.
> (It's not like there's not $BIGNUM ways for a C-language module to
> break the backend, anyway.)

Agreed.  Similarly the process_shared_preload_libraries_in_progress flag could
be modified by extension, and that wouldn't be any better.

> BTW, I'd make such errors FATAL, as it's unlikely that we can recover
> cleanly from an error during initialization of a loadable module.
> The module's likely to be only partially initialized/hooked in.

While at it, should we make process_shmem_requests_in_progress true when the
new hook is called?  The hook should only be called when that's the case, and
extension authors may feel like asserting it.

---

From: "Anton A. Melnikov" <[email-masked]>
Date: Fri, 6 May 2022 09:35:05 +0300

Hello!

On 19.04.2022 18:46, Nathan Bossart wrote:
> Okay, I did it this way in v5.
> 

Recently i ran into a problem that it would be preferable in our 
extended pg_stat_statements to use MaxBackEnds in _PG_Init() but it's 
equal to zero here.
And i was very happy to find this patch and this thread.  It was not 
only very interesting and informative for me but also solves the current 
problem. Patch is applied cleanly on current master. Tests under Linux 
and Windows were successful.
I've tried this patch with our extended pg_stat_statements and checked 
that MaxBackends has the correct value during extra shmem allocating. 
Thanks a lot!

+1 for this patch.

I have a little doubt about the comment in postmaster.c: "Now that 
loadable modules have had their chance to alter any GUCs". Perhaps this 
comment is too general. Not sure that it is possible to change any 
arbitrary GUC here.
And maybe not bad to add Assert(size > 0) in RequestAddinShmemSpace() to 
see that the size calculation in contrib was wrong.

With best regards,

-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

---

From: Robert Haas <[email-masked]>
Date: Fri, 6 May 2022 09:51:09 -0400

> Okay, I did it this way in v5.

I pushed 0001. Regarding 0002, I think there's no point in adding a
_PG_fini(). The code to call _PG_fini() has been surrounded by #ifdef
NOT_USED forever, and that seems unlikely to change any time soon as
long as we stick with these stupid hook designs where there's
effectively a linked list of hooks, but you can't actually access the
list because spread across a bunch of random static variables in each
module. I think we ought to go through all the hooks that are being
used in this kind of way and replace them with a system where there's
an explicit List of functions to call, and instead of this lame stuff
like:

+       prev_shmem_request_hook = shmem_request_hook;
+       shmem_request_hook = autoprewarm_shmem_request;

You instead do:

shmem_request_functions = lappend(shmem_request_functions,
autoprewarm_shmem_request);

Or maybe wrap that up in an API, like:

add_shmem_request_function(autoprewarm_shmem_request);

Then it would be easy to have corresponding a corresponding remove function.

For shmem request, it would probably make sense to ditch the part
where each hook function calls the next hook function and instead just
have the calling code loop over the list and call them one by one. For
things like the executor start/run/end hooks, that wouldn't work, but
you could have something like
invoke_next_executor_start_hook_function(args).

I don't think we should try to make this kind of change now - it seems
to me that what you've done here is consistent with existing practice
and we might as well commit it more or less like this for now. But
perhaps for v16 or some future release we can think about redoing a
bunch of hooks this way. There would be some migration pain for
extension authors for sure, but then we might get to a point where
extensions can be cleanly unloaded in at least some circumstances,
instead of continuing to write silly _PG_fini() functions that
couldn't possibly ever work.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Tom Lane <[email-masked]>
Date: Fri, 06 May 2022 09:57:42 -0400

> couldn't possibly ever work.

I agree that _PG_fini functions as they stand are worthless.
What I'm not getting is why we should care enough about that
to break just about everybody's extension.  Even if unloading
extensions were feasible, who would bother?

			regards, tom lane

---

From: Robert Haas <[email-masked]>
Date: Fri, 6 May 2022 10:10:10 -0400

> extensions were feasible, who would bother?

Well, if we think that, then we ought to remove the NOT_USED code and
all the random _PG_fini() stuff that's still floating around.

I don't actually have a clear answer to whether it's a useful thing to
be able to unload modules. If the module is just providing a bunch of
SQL-callable functions, it probably isn't. If it's modifying the
behavior of your session, it might be. Now, it could also have an
"off" switch in the form of a GUC, and probably should - but you'd
probably save at least a few cycles by detaching from the hooks rather
than just still getting called and doing nothing, and maybe that's
worth something. Whether it's worth enough to justify making changes
that will affect extensions, I'm not sure.

IOW, I don't really know what we ought to do here, but I think that
maintaining a vestigial system that has never worked and can never
work is not it.

---

From: Tom Lane <[email-masked]>
Date: Fri, 06 May 2022 10:43:21 -0400

> all the random _PG_fini() stuff that's still floating around.

I think that's exactly what we should do, if it bugs you that stuff
is just sitting there.  I see no prospect that we'll ever make it
work, because the question of unhooking from hooks is just the tip
of the iceberg.  As an example, what should happen with any custom
GUCs the module has defined?  Dropping their values might not be
very nice, but if we leave them around then the next LOAD (if any)
will see a conflict.  Another fun question is whether it's ever
safe to unload a module that was preloaded by the postmaster.

In short, this seems like a can of very wriggly worms, with not
a lot of benefit that would ensue from opening it.

			regards, tom lane

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 6 May 2022 08:27:11 -0700

> a lot of benefit that would ensue from opening it.

+1, I'll put together a new patch set.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 6 May 2022 14:15:21 -0700

> +1, I'll put together a new patch set.

As promised...

---

From: Robert Haas <[email-masked]>
Date: Fri, 6 May 2022 19:51:43 -0400

> > +1, I'll put together a new patch set.

Looks reasonable to me.

Anyone else have thoughts?

---

From: Michael Paquier <[email-masked]>
Date: Tue, 10 May 2022 17:55:12 +0900

>> > +1, I'll put together a new patch set.

0001 looks sensible seen from here with this approach.

> Anyone else have thoughts?

I agree that removing support for the unloading part would be nice to
clean up now on HEAD.  Note that 0002 is missing the removal of one
reference to _PG_fini in xfunc.sgml (<primary> markup).
--
Michael

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 10 May 2022 08:56:28 -0700

> reference to _PG_fini in xfunc.sgml (<primary> markup).

Oops, sorry about that.

---

From: Michael Paquier <[email-masked]>
Date: Wed, 11 May 2022 11:18:29 +0900

>> reference to _PG_fini in xfunc.sgml (<primary> markup).

0002 looks pretty good from here.  If it were me, I would apply 0002
first to avoid the extra tweak in pg_stat_statements with _PG_fini in
0001.

Regarding 0001, I have spotted an extra issue in the documentation.
xfunc.sgml has a section called "Shared Memory and LWLocks" about the
use of RequestNamedLWLockTranche() and RequestAddinShmemSpace() called
from _PG_init(), which would now be wrong as we'd need the hook.  IMO,
the docs should mention the registration of the hook in _PG_init(),
the APIs involved, and should mention to look at pg_stat_statements.c
for a code sample (we tend to avoid the addition of sample code in the
docs lately as we habe no way to check their compilation
automatically).

Robert, are you planning to look at what's proposed and push these?
FWIW, I am familiar enough with the problems at hand to do this in
time for beta1 (aka by tomorrow to give it room in the buildfarm), but
I don't want to step on your feet, either.
--
Michael

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 10 May 2022 21:12:11 -0700

> first to avoid the extra tweak in pg_stat_statements with _PG_fini in

I swapped 0001 and 0002 in v8.

> docs lately as we habe no way to check their compilation

Ah, good catch.  I adjusted the docs as you suggested.

---

From: Robert Haas <[email-masked]>
Date: Wed, 11 May 2022 16:18:10 -0400

> Ah, good catch.  I adjusted the docs as you suggested.

OK, I have committed 0001 now.

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 11 May 2022 14:01:26 -0700

> OK, I have committed 0001 now.

Thanks!

---

From: "Anton A. Melnikov" <[email-masked]>
Date: Thu, 12 May 2022 18:51:59 +0300

>> OK, I have committed 0001 now.

Maybe remove the first part from the patchset?
Because now the Patch Tester is giving apply error for the first part 
and can't test the other.
http://cfbot.cputube.org/patch_38_3614.log

With best regards,

---

From: Michael Paquier <[email-masked]>
Date: Fri, 13 May 2022 09:15:15 +0900

On Thu, May 12, 2022 at 06:51:59PM +0300, Anton A. Melnikov wrote:
> Because now the Patch Tester is giving apply error for the first part and
> can't test the other.

Yep.  As simple as the attached.
--
Michael
From d69844e34049d8c38b1e05fb75104469b2d123e1 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Mon, 18 Apr 2022 15:25:37 -0700
Subject: [PATCH v9] Add a new shmem_request_hook hook.

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.

Authors: Julien Rouhaud, Nathan Bossart
Discussion: https://postgr.es/m/20220412210112.GA2065815%40nathanxps13
---
 src/include/miscadmin.h                       |  5 ++++
 src/backend/postmaster/postmaster.c           |  5 ++++
 src/backend/storage/ipc/ipci.c                | 20 +++++---------
 src/backend/storage/lmgr/lwlock.c             | 18 ++++---------
 src/backend/utils/init/miscinit.c             | 15 +++++++++++
 doc/src/sgml/xfunc.sgml                       | 12 +++++++--
 contrib/pg_prewarm/autoprewarm.c              | 17 +++++++++++-
 .../pg_stat_statements/pg_stat_statements.c   | 26 +++++++++++++------
 src/tools/pgindent/typedefs.list              |  1 +
 9 files changed, 81 insertions(+), 38 deletions(-)

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 53fd168d93..0af130fbc5 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -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);
 
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index bf591f048d..3b73e26956 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -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
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 75e456360b..26372d95b3 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -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 */
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index fef462b110..8aef909037 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -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)
 	{
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 6ed584e114..ec6a61594a 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -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)
 {
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index fbe718e3c2..3b0adc0704 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -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
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index 45e012a63a..c0c4f5d9ca 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -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.
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 5c8b9ff943..768cedd91a 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -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.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index dd1214977a..4fb746930a 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -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
-- 
2.36.0

---

From: Robert Haas <[email-masked]>
Date: Fri, 13 May 2022 09:49:54 -0400

> Yep.  As simple as the attached.

Committed.

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 13 May 2022 08:35:31 -0700

Thanks!

---

From: Julien Rouhaud <[email-masked]>
Date: Sun, 15 May 2022 20:58:42 +0800

Thanks!

For the record I submitted patches or pull requests this weekend for all
repositories I was aware of to fix the compatibility with this patch, hoping
that it will save some time to the packagers when they will take care of the
beta.

---

From: Robert Haas <[email-masked]>
Date: Mon, 16 May 2022 09:25:22 -0400

> that it will save some time to the packagers when they will take care of the

Nice!