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(at)gmail(dot)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).