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

Hi,

On 2019-09-30 15:13:18 -0400, Mike Palmiotto wrote:
> Attached is the reworked and rebased patch set. I put the hook on top
> and a separate commit for each process type. Note that avworker and
> avlauncher were intentionally left together. Let me know if you think
> those should be split out as well.

I've not looked at this before, nor the thread. Just jumping here
because of the reference to this patch you made in another thread...

In short, I think this is far from ready, nor am I sure this is going in
the right direction. There's a bit more detail about where I think this
ought to go interspersed below (in particular search for PgSubProcess,
EXEC_BACKEND).



> From 205ea86dde898f7edac327d46b2b43b04721aadc Mon Sep 17 00:00:00 2001
> From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
> Date: Mon, 30 Sep 2019 14:42:53 -0400
> Subject: [PATCH 7/8] Add backends to process centralization

.oO(out of order attached patches, how fun). 7, 8, 6, 5, 4, 2, 3, 1...



> From 2a3a35f2e80cb2badcb0efbce1bad2484e364b7b Mon Sep 17 00:00:00 2001
> From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
> Date: Fri, 27 Sep 2019 12:28:19 -0400
> Subject: [PATCH 1/8] Add ForkProcType infrastructure

A better explanation would be nice. It's hard to review non-trivial
patches that have little explanation as to what they're
achieving. Especially when there's some unrelated seeming changes.


> diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
> index 9238fbe98d..9f3dad1c6d 100644
> --- a/src/backend/bootstrap/bootstrap.c
> +++ b/src/backend/bootstrap/bootstrap.c
> @@ -70,6 +70,7 @@ static void cleanup(void);
>   */
>
>  AuxProcType MyAuxProcType = NotAnAuxProcess;    /* declared in miscadmin.h */
> +ForkProcType MyForkProcType = NoForkProcess;    /* declared in fork_process.h */
>
>  Relation    boot_reldesc;        /* current relation descriptor
>  */

IMO, before this stuff gets massaged meaningfully, we ought to move code
like this (including AuxiliaryProcessMain etc) somewhere where it makes
sense. If we're going to rejigger things, it ought to be towards a more
understandable architecture, rather than keeping weird quirks around.

I'm inclined to think that this better also include properly
centralizing a good portion of the EXEC_BACKEND code. Building
infrastructure that's supposed to be long-lived with that being left as
is just seems like recipe for yet another odd framework.


> @@ -538,11 +539,11 @@ static void ShmemBackendArrayAdd(Backend *bn);
>  static void ShmemBackendArrayRemove(Backend *bn);
>  #endif                            /* EXEC_BACKEND */
>
> -#define StartupDataBase()        StartChildProcess(StartupProcess)
> -#define StartBackgroundWriter() StartChildProcess(BgWriterProcess)
> -#define StartCheckpointer()        StartChildProcess(CheckpointerProcess)
> -#define StartWalWriter()        StartChildProcess(WalWriterProcess)
> -#define StartWalReceiver()        StartChildProcess(WalReceiverProcess)
> +#define StartupDataBase()        StartChildProcess(StartupFork)
> +#define StartBackgroundWriter() StartChildProcess(BgWriterFork)
> +#define StartCheckpointer()        StartChildProcess(CheckpointerFork)
> +#define StartWalWriter()        StartChildProcess(WalWriterFork)
> +#define StartWalReceiver()        StartChildProcess(WalReceiverFork)

FWIW, I'd just rip this out, it adds exactly nothing but one more place
to change.


> +/*
> + * PrepAuxProcessFork
> + *
> + * Prepare a ForkProcType struct for the auxiliary process specified by

ForkProcData, not ForkProcType


> + * AuxProcType. This does all prep related to av parameters and error messages.
> + */
> +static void
> +PrepAuxProcessFork(ForkProcData *aux_fork)
> +{
> +    int            ac = 0;
> +
> +    /*
> +     * Set up command-line arguments for subprocess
> +     */
> +    aux_fork->av[ac++] = pstrdup("postgres");

There's no documentation as to what 'av' is actually supposed to mean. I
assume auxvector or such, but if so that's neither obvious, nor
necessarily good.


> +#ifdef EXEC_BACKEND
> +    aux_fork->av[ac++] = pstrdup("--forkboot");
> +    aux_fork->av[ac++] = NULL;            /* filled in by postmaster_forkexec */
> +#endif

What's the point of pstrdup()ing constant strings here? Afaict you're
not freeing them ever?


> +    aux_fork->av[ac++] = psprintf("-x%d", MyForkProcType);
> +
> +    aux_fork->av[ac] = NULL;
> +    Assert(ac < lengthof(*aux_fork->av));

Unless I miss something, the lengthof here doesn't do anythign
useful. ForkProcData->av is defined as:

> +typedef struct ForkProcData
> +{
> +   char                   **av;

which I think means you're computing sizeof(char*)/sizeof(char)?



> +    aux_fork->ac = ac;
> +    switch (MyForkProcType)
> +    {
> +        case StartupProcess:
> +            aux_fork->type_desc = pstrdup("startup");
> +            break;
> +        case BgWriterProcess:
> +            aux_fork->type_desc = pstrdup("background writer");
> +            break;
> +        case CheckpointerProcess:
> +            aux_fork->type_desc = pstrdup("checkpointer");
> +            break;
> +        case WalWriterProcess:
> +            aux_fork->type_desc = pstrdup("WAL writer");
> +            break;
> +        case WalReceiverProcess:
> +            aux_fork->type_desc = pstrdup("WAL receiver");
> +            break;
> +        default:
> +            aux_fork->type_desc = pstrdup("child");
> +            break;
> +    }
> +
> +    aux_fork->child_main = AuxiliaryProcessMain;

I'm really not in love in having details like the descriptors of
processes, the types of processes, the mapping from MyForkProcType to
AuxProcType in multiple places.  I feel this is adding even more
confusion to this, rather than reducing it.

I feel like we should define the properties of the different types of
processes in *one* place, rather multiple. Define one array of
structs that contains information about the different types of
processes, rather than distributing that knowledge all over.


> @@ -4477,11 +4530,11 @@ BackendRun(Port *port)
>  pid_t
>  postmaster_forkexec(int argc, char *argv[])
>  {
> -    Port        port;
> -
>      /* This entry point passes dummy values for the Port variables */
> -    memset(&port, 0, sizeof(port));
> -    return internal_forkexec(argc, argv, &port);
> +    if (!ConnProcPort)
> +        ConnProcPort = palloc0(sizeof(*ConnProcPort));
> +
> +    return internal_forkexec(argc, argv);
>  }

What's the point of creating a dummy ConnProcPort? And when is this
being pfreed? I mean previously this at least used a stack variable that
was automatically being released by the time postmaster_forkexec
returned, now it sure looks like a permanent leak to me.

Also, why is it a good idea to rely *more* on data being implicitly
passed by stuffing things into global variables? That strikes me as


>          /* in parent, fork failed */
> -        int            save_errno = errno;
> +        int save_errno = errno;
>
> -        errno = save_errno;
> -        switch (type)
> -        {
> -            case StartupProcess:
> -                ereport(LOG,
> -                        (errmsg("could not fork startup process: %m")));
> -                break;
> -            case BgWriterProcess:
> -                ereport(LOG,
> -                        (errmsg("could not fork background writer process: %m")));
> -                break;
> -            case CheckpointerProcess:
> -                ereport(LOG,
> -                        (errmsg("could not fork checkpointer process: %m")));
> -                break;
> -            case WalWriterProcess:
> -                ereport(LOG,
> -                        (errmsg("could not fork WAL writer process: %m")));
> -                break;
> -            case WalReceiverProcess:
> -                ereport(LOG,
> -                        (errmsg("could not fork WAL receiver process: %m")));
> -                break;
> -            default:
> -                ereport(LOG,
> -                        (errmsg("could not fork process: %m")));
> -                break;
> -        }
> +        errno =  save_errno;
> +        ereport(LOG,
> +            (errmsg("could not fork %s process: %m", fork_data->type_desc)));

Previously the process type was translatable, now it is not
anymore. You'd probably have to solve that somehow, perhaps by
translating the type_desc once, when setting it.


> @@ -1113,7 +1113,7 @@ process_startup_options(Port *port, bool am_superuser)
>          av = (char **) palloc(maxac * sizeof(char *));
>          ac = 0;
>
> -        av[ac++] = "postgres";
> +        av[ac++] = pstrdup("postgres");
>
>          pg_split_opts(av, &ac, port->cmdline_options);

Again, what's with all these conversions to dynamic memory allocations?



>  /* Flags to tell if we are in an autovacuum process */
> -static bool am_autovacuum_launcher = false;
> -static bool am_autovacuum_worker = false;
> +bool am_autovacuum_launcher = false;
> +bool am_autovacuum_worker = false;

I'm against exposing a multitude of am_* variables for each process
type, especially globally.  If we're building proper infrastructure
here, why do we need multiple variables, rather than exactly one
indicating the process type?



> @@ -347,88 +343,53 @@ static void avl_sigusr2_handler(SIGNAL_ARGS);
>  static void avl_sigterm_handler(SIGNAL_ARGS);
>  static void autovac_refresh_stats(void);
>
> -
> -
>  /********************************************************************
>   *                      AUTOVACUUM LAUNCHER CODE
>   ********************************************************************/

Spurious whitespace changes (in a few other places too).


> +void
> +PrepAutoVacProcessFork(ForkProcData *autovac_fork)
>  {

I'm really not in love with keeping "fork" in all these, given that
we're not actually going to fork in some cases. Why do we not just call
it subprocess and get rid of the "fork" and "forkexec" suffixes?




> +/*
> + * shmemSetup
> + *
> + * Helper function for a child to set up shmem before
> + * executing.
> + *
> + * aux_process - set to true if an auxiliary process.
> + */
> +static void
> +shmemSetup(bool aux_process)
> +{
> +    /* Restore basic shared memory pointers */
> +    InitShmemAccess(UsedShmemSegAddr);
> +
> +    /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
> +    if (aux_process)
> +        InitAuxiliaryProcess();
> +    else
> +        InitProcess();
> +
> +    /* Attach process to shared data structures */
> +    CreateSharedMemoryAndSemaphores();
> +}

Not quite convinced this is a useful abstraction.  I'm inclined to think
that this again should use something more like one array of
per-subprocess properties.


Imagine if we had an array like

typedef struct PgSubProcess
{
        const char *name;
        const char *translated_name;
        bool        needs_shmem;
        bool        needs_aux_proc;
        bool        needs_full_proc;
        SubProcessEntryPoint entrypoint;
} PgSubProcess;

PgSubProcess process_types[] = {
    {.name = "startup process", .needs_shmem = true, .needs_aux_proc = true, .entrypoint = StartupProcessMain},
    {.name = "background writer", .needs_shmem = true, .needs_aux_proc = true, .entrypoint = BackgroundWriterMain},
    {.name = "autovacuum launcher", .needs_shmem = true, .needs_aux_proc = true, .entrypoint = AutoVacLauncherMain},
    {.name = "backend", .needs_shmem = true, .needs_full_proc = true, , .entrypoint = PostgresMain},


> From efc6f0531b71847c6500062b5a0fe43331a6ea7e Mon Sep 17 00:00:00 2001
> From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
> Date: Fri, 27 Sep 2019 19:13:53 -0400
> Subject: [PATCH 3/8] Add centralized stat collector
>
> ---
>  src/backend/postmaster/pgstat.c       | 88 ++++++---------------------
>  src/backend/postmaster/postmaster.c   |  5 +-
>  src/include/pgstat.h                  |  4 +-
>  src/include/postmaster/fork_process.h |  1 +
>  4 files changed, 26 insertions(+), 72 deletions(-)
>
> diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
> index 011076c3e3..73d953aedf 100644
> --- a/src/backend/postmaster/pgstat.c
> +++ b/src/backend/postmaster/pgstat.c
> @@ -280,10 +280,6 @@ static instr_time total_func_time;
>   * Local function forward declarations
>   * ----------
>   */
> -#ifdef EXEC_BACKEND
> -static pid_t pgstat_forkexec(void);
> -#endif
> -
>  NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn();
>  static void pgstat_exit(SIGNAL_ARGS);
>  static void pgstat_beshutdown_hook(int code, Datum arg);
> @@ -689,53 +685,26 @@ pgstat_reset_all(void)
>      pgstat_reset_remove_files(PGSTAT_STAT_PERMANENT_DIRECTORY);
>  }
>
> -#ifdef EXEC_BACKEND
>
>  /*
> - * pgstat_forkexec() -
> - *
> - * Format up the arglist for, then fork and exec, statistics collector process
> - */
> -static pid_t
> -pgstat_forkexec(void)
> -{
> -    char       *av[10];
> -    int            ac = 0;
> -
> -    av[ac++] = "postgres";
> -    av[ac++] = "--forkcol";
> -    av[ac++] = NULL;            /* filled in by postmaster_forkexec */
> -
> -    av[ac] = NULL;
> -    Assert(ac < lengthof(av));
> -
> -    return postmaster_forkexec(ac, av);
> -}
> -#endif                            /* EXEC_BACKEND */
> -
> -
> -/*
> - * pgstat_start() -
> + * PrepPgstatCollectorFork
>   *
>   *    Called from postmaster at startup or after an existing collector
>   *    died.  Attempt to fire up a fresh statistics collector.
>   *
> - *    Returns PID of child process, or 0 if fail.
> - *
> - *    Note: if fail, we will be called again from the postmaster main loop.
>   */
> -int
> -pgstat_start(void)
> +void
> +PrepPgstatCollectorFork(ForkProcData *pgstat_fork)
>  {
> +    int                ac = 0;
>      time_t        curtime;
> -    pid_t        pgStatPid;
>
>      /*
>       * Check that the socket is there, else pgstat_init failed and we can do
>       * nothing useful.
>       */
>      if (pgStatSock == PGINVALID_SOCKET)
> -        return 0;
> +        return;
>
>      /*
>       * Do nothing if too soon since last collector start.  This is a safety
> @@ -746,45 +715,20 @@ pgstat_start(void)
>      curtime = time(NULL);
>      if ((unsigned int) (curtime - last_pgstat_start_time) <
>          (unsigned int) PGSTAT_RESTART_INTERVAL)
> -        return 0;
> +        return;
>      last_pgstat_start_time = curtime;
>
> -    /*
> -     * Okay, fork off the collector.
> -     */
>  #ifdef EXEC_BACKEND
> -    switch ((pgStatPid = pgstat_forkexec()))
> -#else
> -    switch ((pgStatPid = fork_process()))
> -#endif
> -    {
> -        case -1:
> -            ereport(LOG,
> -                    (errmsg("could not fork statistics collector: %m")));
> -            return 0;
> -
> -#ifndef EXEC_BACKEND
> -        case 0:
> -            /* in postmaster child ... */
> -            InitPostmasterChild();
> -
> -            /* Close the postmaster's sockets */
> -            ClosePostmasterPorts(false);
> -
> -            /* Drop our connection to postmaster's shared memory, as well */
> -            dsm_detach_all();
> -            PGSharedMemoryDetach();
> -
> -            PgstatCollectorMain(0, NULL);
> -            break;
> +    pgstat_fork->av[ac++] = pstrdup("postgres");
> +    pgstat_fork->av[ac++] = pstrdup("--forkcol");
> +    pgstat_fork->av[ac++] = NULL;            /* filled in by postmaster_forkexec */
>  #endif
>
> -        default:
> -            return (int) pgStatPid;
> -    }
> +    pgstat_fork->ac = ac;
> +    Assert(pgstat_fork->ac < lengthof(*pgstat_fork->av));
>
> -    /* shouldn't get here */
> -    return 0;
> +    pgstat_fork->type_desc = pstrdup("statistics collector");
> +    pgstat_fork->child_main = PgstatCollectorMain;
>  }
>
>  void
> @@ -4425,12 +4369,16 @@ pgstat_send_bgwriter(void)
>   * ----------
>   */
>  NON_EXEC_STATIC void
> -PgstatCollectorMain(int argc, char *argv[])
> +PgstatCollectorMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[])
>  {
>      int            len;
>      PgStat_Msg    msg;
>      int            wr;
>
> +    /* Drop our connection to postmaster's shared memory, as well */
> +    dsm_detach_all();
> +    PGSharedMemoryDetach();
> +
>      /*
>       * Ignore all signals usually bound to some action in the postmaster,
>       * except SIGHUP and SIGQUIT.  Note we don't need a SIGUSR1 handler to
> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
> index 35cd1479b9..37a36387a3 100644
> --- a/src/backend/postmaster/postmaster.c
> +++ b/src/backend/postmaster/postmaster.c
> @@ -549,6 +549,7 @@ static void ShmemBackendArrayRemove(Backend *bn);
>  #define StartWalReceiver()        StartChildProcess(WalReceiverFork)
>  #define StartAutoVacLauncher()        StartChildProcess(AutoVacLauncherFork)
>  #define StartAutoVacWorker()        StartChildProcess(AutoVacWorkerFork)
> +#define pgstat_start()            StartChildProcess(PgstatCollectorFork)
>
>  /* Macros to check exit status of a child process */
>  #define EXIT_STATUS_0(st)  ((st) == 0)
> @@ -5459,7 +5460,9 @@ StartChildProcess(ForkProcType type)
>          case AutoVacWorkerFork:
>              PrepAutoVacProcessFork(fork_data);
>              break;
> -
> +        case PgstatCollectorFork:
> +            PrepPgstatCollectorFork(fork_data);
> +            break;
>          default:
>              break;
>      }
> diff --git a/src/include/pgstat.h b/src/include/pgstat.h
> index fe076d823d..00e95caa60 100644
> --- a/src/include/pgstat.h
> +++ b/src/include/pgstat.h
> @@ -15,6 +15,7 @@
>  #include "libpq/pqcomm.h"
>  #include "port/atomics.h"
>  #include "portability/instr_time.h"
> +#include "postmaster/fork_process.h"
>  #include "postmaster/pgarch.h"
>  #include "storage/proc.h"
>  #include "utils/hsearch.h"
> @@ -1235,10 +1236,11 @@ extern Size BackendStatusShmemSize(void);
>  extern void CreateSharedBackendStatus(void);
>
>  extern void pgstat_init(void);
> -extern int    pgstat_start(void);
>  extern void pgstat_reset_all(void);
>  extern void allow_immediate_pgstat_restart(void);
>
> +extern void PrepPgstatCollectorFork(ForkProcData *pgstat_fork);
> +
>  #ifdef EXEC_BACKEND
>  extern void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn();
>  #endif
> diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h
> index 1f319fc98f..16ca8be968 100644
> --- a/src/include/postmaster/fork_process.h
> +++ b/src/include/postmaster/fork_process.h
> @@ -26,6 +26,7 @@ typedef enum
>     WalReceiverFork,    /* end of Auxiliary Process Forks */
>     AutoVacLauncherFork,
>     AutoVacWorkerFork,
> +   PgstatCollectorFork,
>
>     NUMFORKPROCTYPES            /* Must be last! */
>  } ForkProcType;




> ---
>  src/backend/postmaster/postmaster.c   | 309 +++++++++++++-------------
>  src/include/postmaster/fork_process.h |   5 +
>  src/include/postmaster/postmaster.h   |   1 -
>  3 files changed, 160 insertions(+), 155 deletions(-)
>
> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
> index 8f862fcd64..b55cc4556d 100644
> --- a/src/backend/postmaster/postmaster.c
> +++ b/src/backend/postmaster/postmaster.c
> @@ -144,6 +144,7 @@ static Backend *ShmemBackendArray;
>
>  BackgroundWorker *MyBgworkerEntry = NULL;
>  RegisteredBgWorker *CurrentBgWorker = NULL;
> +static Backend       *MyBackend;
>  static int            child_errno;

This is another global variable that strikes me as a bad idea. Whereas
MyBgworkerEntry is only set in the bgworker itself, you appear to set
MyBackend *in postmaster*:

> +/*
> + * PrepBackendFork
> + *
> + * Prepare a ForkProcType struct for starting a Backend.
> + * This does all prep related to av parameters and error messages.
> +*/
> +static void
> +PrepBackendFork(ForkProcData *backend_fork)
> +{
> +    int            ac = 0;
> +
> +    /*
> +     * Create backend data structure.  Better before the fork() so we can
> +     * handle failure cleanly.
> +     */
> +    MyBackend = (Backend *) malloc(sizeof(Backend));
> +    if (!MyBackend)
> +    {
> +        ereport(LOG,
> +                (errcode(ERRCODE_OUT_OF_MEMORY),
> +                 errmsg("out of memory")));
> +    }

That's totally not ok, we'll constantly overwrite the variable set by
another process, and otherwise have now obsolete values in there. And
even worse, you leave it dangling in some cases:

> +    /*
> +     * Compute the cancel key that will be assigned to this backend. The
> +     * backend will have its own copy in the forked-off process' value of
> +     * MyCancelKey, so that it can transmit the key to the frontend.
> +     */
> +    if (!RandomCancelKey(&MyCancelKey))
> +    {
> +        free(MyBackend);
> +        ereport(LOG,
> +                (errcode(ERRCODE_INTERNAL_ERROR),
> +                 errmsg("could not generate random cancel key")));
> +    }


Nor do I see where you're actually freeing that memory in postmaster, in
the successful case.  Previously it was freed in postmaster in the
success case

> -#ifdef EXEC_BACKEND
> -    pid = backend_forkexec(port);
> -#else                            /* !EXEC_BACKEND */
> -    pid = fork_process();
> -    if (pid == 0)                /* child */
> -    {
> -        free(bn);
> -

but now it appears  it's only freed in the failure case:

> +/*
> + *     BackendFailFork
> + *
> + *     Backend cleanup in case a failure occurs forking a new Backend.
> + */
> +static void BackendFailFork(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[])
> +{
> +    if (!MyBackend->dead_end)
> +        (void) ReleasePostmasterChildSlot(MyBackend->child_slot);
> +    free(MyBackend);
> +
> +    report_fork_failure_to_client(MyProcPort, child_errno);
> +}



> From 718c6598e9e02e1ce9ade99afb3c8948c67ef5ae Mon Sep 17 00:00:00 2001
> From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
> Date: Tue, 19 Feb 2019 15:29:33 +0000
> Subject: [PATCH 8/8] Add a hook to allow extensions to set worker metadata
>
> This patch implements a hook that allows extensions to modify a worker
> process' metadata in backends.
>
> Additional work done by Yuli Khodorkovskiy <yuli@crunchydata.com>
> Original discussion here:
https://www.postgresql.org/message-id/flat/CAMN686FE0OdZKp9YPO%3DhtC6LnA6aW4r-%2Bjq%3D3Q5RAoFQgW8EtA%40mail.gmail.com
> ---
>  src/backend/postmaster/fork_process.c | 11 +++++++++++
>  src/include/postmaster/fork_process.h |  4 ++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/src/backend/postmaster/fork_process.c b/src/backend/postmaster/fork_process.c
> index a9138f589e..964c04d846 100644
> --- a/src/backend/postmaster/fork_process.c
> +++ b/src/backend/postmaster/fork_process.c
> @@ -21,6 +21,14 @@
>  #include <openssl/rand.h>
>  #endif
>
> +/*
> + * This hook allows plugins to get control of the child process following
> + * a successful fork_process(). It can be used to perform some action in
> + * extensions to set metadata in backends (including special backends) upon
> + * setting of the session user.
> + */
> +ForkProcess_post_hook_type ForkProcess_post_hook = NULL;
> +
>  #ifndef WIN32
>  /*
>   * Wrapper for fork(). Return values are the same as those for fork():
> @@ -113,6 +121,9 @@ fork_process(void)
>  #ifdef USE_OPENSSL
>          RAND_cleanup();
>  #endif
> +
> +        if (ForkProcess_post_hook)
> +            (*ForkProcess_post_hook) ();
>      }


>      return result;
> diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h
> index 631a2113b5..d756fcead7 100644
> --- a/src/include/postmaster/fork_process.h
> +++ b/src/include/postmaster/fork_process.h
> @@ -58,4 +58,8 @@ extern pid_t fork_process(void);
>  extern pid_t postmaster_forkexec(ForkProcData *fork_data);
>  #endif
>
> +/* Hook for plugins to get control after a successful fork_process() */
> +typedef void (*ForkProcess_post_hook_type) ();
> +extern PGDLLIMPORT ForkProcess_post_hook_type ForkProcess_post_hook;
> +
>  #endif                            /* FORK_PROCESS_H */
> --
> 2.23.0
>

Why do we want libraries to allow to hook into processes like the
startup process etc?


Greetings,

Andres Freund