Re: make MaxBackends available in _PG_init
Julien Rouhaud <rjuju123@gmail.com>
From: Julien Rouhaud <rjuju123@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Nathan Bossart <nathandbossart@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>, Andres Freund <andres@anarazel.de>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Greg Sabino Mullane <htamfids@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-25T03:11:46Z
Lists: pgsql-hackers
Attachments
- v1-0001-POC-Add-a-new-shmem_request_hook-hook.patch (text/plain)
On Fri, Mar 25, 2022 at 10:39:51AM +0800, Julien Rouhaud wrote: > On Thu, Mar 24, 2022 at 04:27:36PM -0400, Robert Haas wrote: > > On Thu, Mar 24, 2022 at 4:20 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > > > Another possibility could be to add a hook that is called _before_ > > > _PG_init() where libraries are permitted to adjust GUCs. After the library > > > is loaded, we first call this _PG_change_GUCs() function, then we > > > initialize MaxBackends, and then we finally call _PG_init(). This way, > > > extensions would have access to MaxBackends within _PG_init(), and if an > > > extension really needed to alter GUCs, іt could define this new function. > > > > Yeah, I think this might be better. > > Well, if it's before _PG_init() then it won't be a new hook but a new symbol > that has to be handled with dlsym. > > But it seems to be that the bigger problem is that this approach won't fix > anything unless we can prevent third-party code from messing with GUCs after > that point as we could otherwise have discrepancies between GetMaxBackends() > and the underlying GUCs until every single extension that wants to change GUC > is modified uses this new symbol. And I also don't see how we could force that > unless we have a better GUC API that doesn't entirely relies on strings, > otherwise a simple thing like "allow 5 extra bgworkers" is going to be really > painful. > > A new hook after _PG_init() sure leaves the burden to the few extension that > needs to allocate shmem based on MaxBackends, but there probably isn't that > much (I have a few dozens locally cloned and found only 1 apart from mine, and > I would be happy to take care of those), and it also means that they have a > chance to do something correct even if other extensions messing with GUCs > aren't fixed. > > Arguably, you could certainly try to change GUCs in that new hook, but it > wouldn't be any different from doing so in shmem_startup_hook so I don't think > it's really a problem. As an example, here's a POC for a new shmem_request_hook hook after _PG_init(). With it I could easily fix pg_wait_sampling shmem allocation (and checked that it's indeed requesting the correct size).
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