Re: Auxiliary Processes and MyAuxProc - Mailing list pgsql-hackers

On Thu, Mar 19, 2020 at 6:35 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> While working on (My)BackendType, I was attempting to get rid of
> (My)AuxProcType altogether.  This would mostly work except that it's
> sort of wired into the pgstats subsystem (see NumBackendStatSlots).
> This can probably be reorganized, but I didn't pursue it further.

This patchset has a different goal: to remove redundant startup code
and interspersed variants of fork/forkexec code so that we can
centralize the postmaster child startup.

The goal of centralizing postmaster startup stems from the desire to
be able to control the process security attributes immediately
before/after fork/exec. This is simply not possible with the existing
infrastructure, since processes are identified in Main functions,
which is too late (and again too scattered) to be able to do anything
useful.

By providing a mechanism to set child process metadata prior to
spawning the subprocess, we gain the ability to identify the process
type and thus set security attributes on that process.

In an earlier spin of the patchset, I included a fork_process hook,
which would be where an extension could set security attributes on a
process. I have since dropped the fork (as advised), but now realize
that it actually demonstrates the main motivation of the patchset.
Perhaps I should add that back in for the next version.

>
> Now, I'm a sucker for refactoring, but I feel this proposal is going
> into a direction I don't understand.  I'd prefer that we focus around
> building out background workers as the preferred subprocess mechanism.
> Building out a second generic mechanism, again, I don't understand the
> direction.  Are we hoping to add more of these processes?  Make it
> extensible?  The net lines added/removed by this patch series seems
> pretty neutral.  What are we getting at the end of this?

As stated above, the primary goal is to centralize the startup code.
One nice side-effect is the introduction of a mechanism that is now
both extensible and provides the ability to remove a lot of redundant
code. I see no reason to have 5 different variants of process forkexec
functions for the sole purpose of building up argv. This patchset
intends to get rid of such an architecture.

Note that this is not intended to be the complete product here -- it
is just a first step at swapping in and making use of a new
infrastructure. There will be follow-up work required to really get
the most out of this infrastructure. For instance, we could drop a
large portion of the SubPostmasterMain switch logic. There are a
number of other areas throughout the codebase (including the example
provided in the last commit, which changes the way we retrieve process
descriptions), that can utilize this new infrastructure to get rid of
code.

>
> More specifically, I don't agree with the wholesale renaming of
> auxiliary process to subprocess.  Besides the massive code churn, the

I'm not sure I understand the argument here. Where do you see
wholesale renaming of AuxProc to Subprocess? Subprocess is the name
for postmaster subprocesses, whereas Auxiliary Processes are a subset
of those processes.

> old naming seemed pretty good to distinguish them from background
> workers, the new naming is more ambiguous.

I do not see any conflation between Background Workers and Auxiliary
Processes in this patchset. Since Auxiliary Processes are included in
the full set of subprocesses and are delineated with a boolean:
needs_aux_proc, it seems fairly straightforward to me which
subprocesses are in fact Auxiliary Processes.

Thanks,
-- 
Mike Palmiotto
https://crunchydata.com