Re: make MaxBackends available in _PG_init - Mailing list pgsql-hackers

Hi,

On 2021-08-02 21:57:18 +0000, Bossart, Nathan wrote:
> On 8/2/21, 1:37 PM, "Andres Freund" <andres@anarazel.de> wrote:
> > On 2021-08-02 16:06:15 -0400, Robert Haas wrote:
> >> I think this is a good idea. Anyone object?
> >
> > I'm not so sure it's a good idea. I've seen several shared_preload_library
> > using extensions that adjust some GUCs (e.g. max_prepared_transactions)
> > because they need some more resources internally - that's perhaps not a great
> > idea, but there's also not an obviously better way.
> 
> Interesting.  I hadn't heard of extensions adjusting GUCs in
> _PG_init() before.  I think the other way to handle that scenario is
> to check the GUCs in _PG_init() and fail startup if necessary.

The problem is that that makes it hard to configure things for the users own
needs. If e.g. the user's workload needs a certain number of prepared
transactions and the extension internally as well (as e.g. citus does), the
extension can't know if the configured number is "aimed" to be for the
extension, or for the user's own needs. If you instead just increase
max_prepared_transactions in _PG_init(), the story is different.


> However, while it's probably good to avoid changing GUCs from what
> users specified, failing startup isn't exactly user-friendly, either.
> In any case, changing a GUC in _PG_init() seems quite risky.  You're
> effectively bypassing all of the usual checks.

It doesn't need to bypass all - you can override them using GUC mechanisms at
that point.


> > ISTM this would better be solved with making the hook or config logic for
> > libraries a bit more elaborate (e.g. having a hook you can register for that's
> > called once all libraries are loaded).
> 
> My worry is that this requires even more specialized knowledge to get
> things right.  If I need to do anything based on the value of a GUC,
> I have to register another hook.  In this new hook, we'd likely want
> to somehow prevent modules from further adjusting GUCs.  We'd also
> need modules to check the GUCs they care about again in case another
> module's _PG_init() adjusted it in an incompatible way.

I think this is overblown. We already size resources *after*
shared_preload_libraries' _PG_init() runs, because that's the whole point of
shared_preload_libraries. What's proposed in this thread is to *disallow*
increasing resource usage in s_p_l _PG_init(), to make one specific case
simpler - but it'll actually also make things more complicated, because other
resources will still only be sized after all of s_p_l has been processed.


> If you detect a problem in this new hook, you probably need to fail startup.

Same is true for _PG_init().


> Perhaps I am making a mountain out of a molehill and the modules that
> are adjusting GUCs in _PG_init() are doing so in a generally safe way,
> but IMO it ought to ordinarily be discouraged.

It's not something I would recommend doing blithely - but I also don't think
we have a better answer for some cases.

Greetings,

Andres Freund