Re: make MaxBackends available in _PG_init

On Fri, Jan 28, 2022 at 9:19 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> As of the issues of this thread, we really have two things to think
> about:
> 1) How do we want to control the access of some parameters in a
> context or another? One idea would be more control through GUCs, say
> with a set of context-related flags that prevent the read of some
> variables until they are set. We could encourage the use of
> GetConfigOption() for that. For MaxBackends, we could add a read-only
> GUC for this purpose. That's what Andres hinted at upthread, I
> guess.
> 2) How do we deal with unwanted access of shared parameters? This one
> is not really controllable, is it? And we are talking about much more
> than MaxBackends. This could perhaps be addressed with more
> documentation in the headers for the concerned variables, as a first
> step.

I think you are mischaracterizing the problem. MaxBackends is not
max_connections. It is a value which is computed based on the value of
max_connections and various other GUCs. And that's the problem. If you
have a C variable whose value should depend on a single GUC, the GUC
system will generally do what you want automatically. If it doesn't,
you can use the GUC check hook/assign hook machinery to update as many
global variables as you like whenever the GUC itself is updated.
Moreover, a GUC always has some legal and thus halfway reasonable
value. We read postgresql.conf super-early in the startup sequence,
because we know GUCs are super-important, and even before that
settings have bootstrap values which are not usually totally insane.
The bottom line is that if you have a global variable whose value
depends on one GUC, you can probably just read that global variable
and call it good.

The main reason why this doesn't work for MaxBackends is that
MaxBackends depends on the values of multiple GUCs. There is a further
wrinkle too, which is that none of those GUCs can change, and
therefore code does things with the resulting value on the assumption
that they won't change, like size shared-memory data structures.
Therefore, if you read the wrong value, you've got a big problem. So
the real issues here IMHO are about the difficulty of making sure that
(1) when a GUC changes, we update all of the things that depend on it
including things which may also depend on other GUCs and (2) making
sure that values which can't ever change are computed before they are
used.

I don't know what the solution to problem #1 is, but the solution to
problem #2 is simple: make people call a function to get the value
rather than just reading a bare variable. GetConfigOption() is not a
good solution for people aiming to write C code that does useful
things, because it delivers the value as a string, and that is not
what you want. But an accessor function like GetMaxBackends() for a
quantity of this type is wonderful. Depending on the situation, you
might choose to have the accessor function [a] fail an assertion if
the value is not available yet or [b] compute the value if they value
has not yet been computed or [c] do the latter if possible, otherwise
the former. But the fact that you are making code call a function
rather than just read a variable gives you a very strong tool to make
sure that someone can't blindly read a 0 or whatever instead of the
real value.

Therefore, it is my opinion that the proposed patch was pretty much
right on the money and that we ought to get it committed.

--
Robert Haas
EDB: http://www.enterprisedb.com