Re: make MaxBackends available in _PG_init
Robert Haas <robertmhaas@gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, 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-05-06T13:51:09Z
Lists: pgsql-hackers
On Tue, Apr 19, 2022 at 11:47 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > 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
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