Re: make MaxBackends available in _PG_init
Julien Rouhaud <rjuju123@gmail.com>
From: Julien Rouhaud <rjuju123@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, "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>, Tom Lane <tgl@sss.pgh.pa.us>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-27T04:15:25Z
Lists: pgsql-hackers
On Sat, Mar 26, 2022 at 10:23:16AM -0700, Andres Freund wrote: > > On 2022-03-26 15:22:03 +0800, Julien Rouhaud wrote: > > On Fri, Mar 25, 2022 at 03:23:17PM -0700, Andres Freund wrote: > > > > > > I don't really understand. The issue that started this thread was bugs in > > > extensions due to accessing MaxBackends before it is initialized - which the > > > patch prevents. > > > > Well, the patch prevents accessing a 0-valued MaxBackends but doesn't do > > anything to solve the original complaint. It's not like extensions won't need > > to access that information during _PG_init anymore. > > It resolves the pretty common bug that an extension breaks once it's used via > s_p_l instead of loaded on-demand because MaxBackends isn't initialized in the > s_p_l case. I can hear the argument. However I don't know any extension that relies on MaxBackends and doesn't need to be in s_p_l, and unless I'm missing something no one provided such an example, only people needing the value for RequestAddinShmemSpace(). > > And indeed, any third party code that previously needed to access what > > MaxBackends is supposed to store should already be using that formula, and > > the new GetMaxBackends() doesn't do anything about it. > > It couldn't rely on MaxBackends before. It can't rely on GetMaxBackends() > now. You can see why I think that what you want is unrelated to the > introduction of GetMaxBackends(). Sure, but code also couldn't really rely on MaxConnections or any other similar GUCs and yet nothing is done for that, and the chosen approach doesn't help for that. The only difference is that those GUCs are less broken, as in the value is likely to be valid more often, and closer to the final value when it's not. But still broken. I think GetMaxBackends() is more likely to force authors to rely on computing the value using the underlying GUCs, since there's nothing else that can be done. And if that's an acceptable answer, why aren't we computing MaxBackends before and after processing s_p_l? > If we introduce a separate hook that allows to influence things like > max_connections or whatnot we'd *even more* need a way to verify whether it's > legal to access MaxBackends in that moment. Again, I'm not opposed to validating that MaxBackends is valid when third-party code is accessing it, quite the opposite. I'm opposed to do so *only* for MaxBackends, and without providing a way for third-party code to access that value when they need it, which is for all the cases I know (after more digging that's now 5 extensions) for RequestAddinShmemSpace().
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