Re: make MaxBackends available in _PG_init
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>,
Andres Freund <andres@anarazel.de>,
Julien Rouhaud <rjuju123@gmail.com>,
Michael Paquier <michael@paquier.xyz>,
"Bossart,
Nathan" <bossartn@amazon.com>,
Fujii Masao <masao.fujii@oss.nttdata.com>,
"wangsh.fnst@fujitsu.com" <wangsh.fnst@fujitsu.com>,
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
Greg Sabino Mullane <htamfids@gmail.com>,
"pgsql-hackers@lists.postgresql.org"
<pgsql-hackers@lists.postgresql.org>
Date: 2022-04-13T16:05:08Z
Lists: pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> What's a little wonky right now is that it's fairly common for
> extensions to just return straight-off without doing *anything at all*
> if !process_shared_preload_libraries_in_progress. See
> pg_stat_statements for example. It seems kind of strange to me to make
> registering GUCs dependent on
> process_shared_preload_libraries_in_progress; why shouldn't that just
> happen always? It's understandable that we don't want to install the
> hook functions if we're not being loaded from shared_preload_libaries,
> though.
Yeah, I was just investigating that. The problem that pg_stat_statements
has is that it wants to create PGC_POSTMASTER GUCs, which is something
that guc.c specifically forbids:
/*
* Only allow custom PGC_POSTMASTER variables to be created during shared
* library preload; any later than that, we can't ensure that the value
* doesn't change after startup. This is a fatal elog if it happens; just
* erroring out isn't safe because we don't know what the calling loadable
* module might already have hooked into.
*/
if (context == PGC_POSTMASTER &&
!process_shared_preload_libraries_in_progress)
elog(FATAL, "cannot create PGC_POSTMASTER variables after startup");
The key reason guc.c does that is that if it allowed the case, then there
would be no guarantee that a "PGC_POSTMASTER" GUC has the same value in
every backend of the cluster, which'd likely break most use-cases for
such a GUC (it'd certainly break pg_stat_statements, which assumes that
the local setting of that GUC reflects the size of its shmem area).
Perhaps we can improve on that situation with some more thought, but
I'm not very clear on how.
> 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.
regards, tom lane
Commits
-
Add a new shmem_request_hook hook.
- 4f2400cb3f10 15.0 landed
-
Remove non-functional code for unloading loadable modules.
- ab02d702ef08 15.0 landed
-
Fix misleading comments about background worker registration.
- 701d918a426b 15.0 landed
-
Revert the addition of GetMaxBackends() and related stuff.
- 7fc0e7de9fb8 15.0 landed
-
Fix typo in multixact.c
- 0147fc7c8c92 15.0 landed
-
Reduce more the number of calls to GetMaxBackends()
- 4567596316d1 15.0 landed
-
Remove MaxBackends variable in favor of GetMaxBackends() function.
- aa64f23b0292 15.0 landed
-
Fix comments about bgworker registration before MaxBackends initialization
- 5ecd0183fb6a 15.0 landed
-
Fix bogus assertion in BootstrapModeMain().
- e12694523e7e 15.0 cited
-
Make sure MaxBackends is always set
- dfbba2c86cc8 9.3.0 cited