Re: make MaxBackends available in _PG_init

On Wed, Apr 13, 2022 at 9:27 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > Hmm. I suppose I was thinking that we'd go the other way around: move
> > the call to InitializeMaxBackends() earlier, as proposed previously,
> > and add a hook to allow extensions to get control before that point.
> > The reason that I like that approach is that I suspect that it's more
> > common for extensions to want to size shared memory data structures
> > than it is for them to want to change GUCs, and therefore I thought
> > that approach would fix things for the most amount of people with the
> > least amount of code change. But it seems like maybe Tom thinks I'm
> > incorrect about the relative frequency of those things, so I don't
> > know.
>
> Maybe I'm missing something, but I figured we'd keep the _PG_init
> calls where they are to minimize side-effects, and then add an optional
> hook just before/after shared memory size is determined. Cases that
> work well now continue to work well, and cases that don't work so
> well can be fixed by making use of the hook. In particular you
> can still do RequestAddinShmemSpace() in _PG_init as long as the
> request size doesn't depend on factors that other extensions might
> change. If you're doing something funny then you might need to
> postpone RequestAddinShmemSpace till the new hook call.

I wouldn't say that approach is wrong. I think we're just prioritizing
different things. I think that the pattern of wanting to request
shared memory in _PG_init() is common, and there are lots of
extensions that already do it that way, and if we pursue this plan,
then all of those extensions really ought to be updated to use the new
hook, including pg_prewarm and pg_stat_statements. They may continue
to work if they don't, but they'll be doing something that is not
really best practice any more but will happen to work until you make
your request for shared memory dependent on the wrong thing, or until
you load up another module at the same time that tweaks some GUC upon
which your calculation depends (imagine an autotuner that adjusts
pg_stat_statements.max at startup time). So I think we'll be
inflicting subtle bugs on extension authors that will never really get
fully sorted out.

Now, on the other hand, I think that the pattern of changing GUCs in
_PG_init() is comparatively uncommon. I don't believe that we have any
in-core code that does it, and the only out-of-core code that does to
my knowledge is Citus. So if we subtly change the best practices
around setting GUCs in _PG_init() hooks, I think that's going to
affect a vastly smaller number of extensions. Either way, we're
changing best practices without really creating a hard break, but I'd
rather move the wood for the small number of extensions that are
tweaking GUCs in _PG_init() than the larger number of extensions (or
so I suppose) that are requesting shared memory there.

Now, we could also decide to create a hard compatibility break to
force extension authors to update. I think that could be done in one
of two ways. One possibility is to refuse SetConfigOption() in
_PG_init(); anyone who wants to do that has to use the new set-a-GUC
hook we simultaneously add. The other is to refuse
RequestAddinShmemSpace() and RequestNamedLWLockTranche() in
_PG_init(); anyone who wants to do that has to use the new
request-shmem-resources hook that we simultaneously add. I tend to
think that the latter is a slightly more principled option, because I
doubt that refusing SetConfigOption() is a bullet-proof defense
against, well, I don't know, anything really. You can hackily modify
GUCs by other means, and you can do bad math with things that aren't
GUCs. I am however pretty sure that if we refuse to provide shmem
space unless you request it from a new add-shmem hook, that will be
pretty water-tight, and it doesn't seem like it should be a big issue
for extension authors to move that code to some new hook we invent.

So in the end I see basically four possibilities here:

A. No hard compatibility break, but invent a set-a-GUC hook that runs earlier.
B. No hard compatibility break, but invent a request-shmem hook that runs later.
C. Invent a set-a-GUC hook that runs earlier AND refuse to set GUCs in _PG_init.
D. Invent a request-shmem hook that runs later AND refuse to accept
shmem requests in _PG_init.

My preferred options are A or D for the reasons explained above. It
seems like you prefer B.

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