process startup: Separate out BootstrapModeMain from AuxiliaryProcessMain.

There practically was no shared code between the two, once all the ifs are
removed. And it was quite confusing that aux processes weren't actually
started by the call to AuxiliaryProcessMain() in main().

There's more to do, AuxiliaryProcessMain() should move out of bootstrap.c, and
BootstrapModeMain() shouldn't use/be part of AuxProcType.

Author: Andres Freund 
Reviewed-By: Kyotaro Horiguchi 
Reviewed-By: Robert Haas 
Discussion: https://postgr.es/m/20210802164124[email protected]

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 67cd5ac6e9ad7aaec0dabc9a5f03ab483940878d..7d6a9d7448b6f8ef0cbd60f77243ecda757b90e6 100644 (file)
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -55,7 +55,6 @@ uint32        bootstrap_data_checksum_version = 0;    /* No checksum */
 
 
 static void CheckerModeMain(void);
-static void BootstrapModeMain(void);
 static void bootstrap_signals(void);
 static void ShutdownAuxiliaryProcess(int code, Datum arg);
 static Form_pg_attribute AllocateAttribute(void);
@@ -194,124 +193,11 @@ static IndexList *ILHead = NULL;
  *  This code is here just because of historical reasons.
  */
 void
-AuxiliaryProcessMain(int argc, char *argv[])
+AuxiliaryProcessMain(AuxProcType auxtype)
 {
-   char       *progname = argv[0];
-   int         flag;
-   char       *userDoption = NULL;
-
-   /*
-    * Initialize process environment (already done if under postmaster, but
-    * not if standalone).
-    */
-   if (!IsUnderPostmaster)
-       InitStandaloneProcess(argv[0]);
-
-   /*
-    * process command arguments
-    */
-
-   /* Set defaults, to be overridden by explicit options below */
-   if (!IsUnderPostmaster)
-       InitializeGUCOptions();
-
-   /* Ignore the initial --boot argument, if present */
-   if (argc > 1 && strcmp(argv[1], "--boot") == 0)
-   {
-       argv++;
-       argc--;
-   }
-
-   /* If no -x argument, we are a CheckerProcess */
-   MyAuxProcType = CheckerProcess;
-
-   while ((flag = getopt(argc, argv, "B:c:d:D:Fkr:x:X:-:")) != -1)
-   {
-       switch (flag)
-       {
-           case 'B':
-               SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV);
-               break;
-           case 'D':
-               userDoption = pstrdup(optarg);
-               break;
-           case 'd':
-               {
-                   /* Turn on debugging for the bootstrap process. */
-                   char       *debugstr;
-
-                   debugstr = psprintf("debug%s", optarg);
-                   SetConfigOption("log_min_messages", debugstr,
-                                   PGC_POSTMASTER, PGC_S_ARGV);
-                   SetConfigOption("client_min_messages", debugstr,
-                                   PGC_POSTMASTER, PGC_S_ARGV);
-                   pfree(debugstr);
-               }
-               break;
-           case 'F':
-               SetConfigOption("fsync", "false", PGC_POSTMASTER, PGC_S_ARGV);
-               break;
-           case 'k':
-               bootstrap_data_checksum_version = PG_DATA_CHECKSUM_VERSION;
-               break;
-           case 'r':
-               strlcpy(OutputFileName, optarg, MAXPGPATH);
-               break;
-           case 'x':
-               MyAuxProcType = atoi(optarg);
-               break;
-           case 'X':
-               {
-                   int         WalSegSz = strtoul(optarg, NULL, 0);
-
-                   if (!IsValidWalSegSize(WalSegSz))
-                       ereport(ERROR,
-                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("-X requires a power of two value between 1 MB and 1 GB")));
-                   SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL,
-                                   PGC_S_OVERRIDE);
-               }
-               break;
-           case 'c':
-           case '-':
-               {
-                   char       *name,
-                              *value;
-
-                   ParseLongOption(optarg, &name, &value);
-                   if (!value)
-                   {
-                       if (flag == '-')
-                           ereport(ERROR,
-                                   (errcode(ERRCODE_SYNTAX_ERROR),
-                                    errmsg("--%s requires a value",
-                                           optarg)));
-                       else
-                           ereport(ERROR,
-                                   (errcode(ERRCODE_SYNTAX_ERROR),
-                                    errmsg("-c %s requires a value",
-                                           optarg)));
-                   }
-
-                   SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
-                   free(name);
-                   if (value)
-                       free(value);
-                   break;
-               }
-           default:
-               write_stderr("Try \"%s --help\" for more information.\n",
-                            progname);
-               proc_exit(1);
-               break;
-       }
-   }
+   Assert(IsUnderPostmaster);
 
-   if (argc != optind)
-   {
-       write_stderr("%s: invalid command-line arguments\n", progname);
-       proc_exit(1);
-   }
+   MyAuxProcType = auxtype;
 
    switch (MyAuxProcType)
    {
@@ -334,47 +220,22 @@ AuxiliaryProcessMain(int argc, char *argv[])
            MyBackendType = B_WAL_RECEIVER;
            break;
        default:
+           elog(ERROR, "something has gone wrong");
            MyBackendType = B_INVALID;
    }
-   if (IsUnderPostmaster)
-       init_ps_display(NULL);
-
-   /* Acquire configuration parameters, unless inherited from postmaster */
-   if (!IsUnderPostmaster)
-   {
-       if (!SelectConfigFiles(userDoption, progname))
-           proc_exit(1);
-   }
 
-   /*
-    * Validate we have been given a reasonable-looking DataDir and change
-    * into it (if under postmaster, should be done already).
-    */
-   if (!IsUnderPostmaster)
-   {
-       checkDataDir();
-       ChangeToDataDir();
-   }
-
-   /* If standalone, create lockfile for data directory */
-   if (!IsUnderPostmaster)
-       CreateDataDirLockFile(false);
+   init_ps_display(NULL);
 
    SetProcessingMode(BootstrapProcessing);
    IgnoreSystemIndexes = true;
 
-   /* Initialize MaxBackends (if under postmaster, was done already) */
-   if (!IsUnderPostmaster)
-       InitializeMaxBackends();
-
    BaseInit();
 
    /*
-    * When we are an auxiliary process, we aren't going to do the full
-    * InitPostgres pushups, but there are a couple of things that need to get
-    * lit up even in an auxiliary process.
+    * As an auxiliary process, we aren't going to do the full InitPostgres
+    * pushups, but there are a couple of things that need to get lit up even
+    * in an auxiliary process.
     */
-   if (IsUnderPostmaster)
    {
        /*
         * Create a PGPROC so we can use LWLocks.  In the EXEC_BACKEND case,
@@ -423,22 +284,9 @@ AuxiliaryProcessMain(int argc, char *argv[])
    switch (MyAuxProcType)
    {
        case CheckerProcess:
-           /* don't set signals, they're useless here */
-           CheckerModeMain();
-           proc_exit(1);       /* should never return */
-
        case BootstrapProcess:
-
-           /*
-            * There was a brief instant during which mode was Normal; this is
-            * okay.  We need to be in bootstrap mode during BootStrapXLOG for
-            * the sake of multixact initialization.
-            */
-           SetProcessingMode(BootstrapProcessing);
-           bootstrap_signals();
-           BootStrapXLOG();
-           BootstrapModeMain();
-           proc_exit(1);       /* should never return */
+           pg_unreachable();
+           break;
 
        case StartupProcess:
            StartupProcessMain();
@@ -490,13 +338,159 @@ CheckerModeMain(void)
  *  The bootstrap backend doesn't speak SQL, but instead expects
  *  commands in a special bootstrap language.
  */
-static void
-BootstrapModeMain(void)
+void
+BootstrapModeMain(int argc, char *argv[])
 {
    int         i;
+   char       *progname = argv[0];
+   int         flag;
+   char       *userDoption = NULL;
 
    Assert(!IsUnderPostmaster);
-   Assert(IsBootstrapProcessingMode());
+
+   InitStandaloneProcess(argv[0]);
+
+   /* Set defaults, to be overridden by explicit options below */
+   InitializeGUCOptions();
+
+   /* an initial --boot should be present */
+   Assert(argc == 1
+          || strcmp(argv[1], "--boot") != 0);
+   argv++;
+   argc--;
+
+   /* If no -x argument, we are a CheckerProcess */
+   MyAuxProcType = CheckerProcess;
+
+   while ((flag = getopt(argc, argv, "B:c:d:D:Fkr:x:X:-:")) != -1)
+   {
+       switch (flag)
+       {
+           case 'B':
+               SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+               break;
+           case 'D':
+               userDoption = pstrdup(optarg);
+               break;
+           case 'd':
+               {
+                   /* Turn on debugging for the bootstrap process. */
+                   char       *debugstr;
+
+                   debugstr = psprintf("debug%s", optarg);
+                   SetConfigOption("log_min_messages", debugstr,
+                                   PGC_POSTMASTER, PGC_S_ARGV);
+                   SetConfigOption("client_min_messages", debugstr,
+                                   PGC_POSTMASTER, PGC_S_ARGV);
+                   pfree(debugstr);
+               }
+               break;
+           case 'F':
+               SetConfigOption("fsync", "false", PGC_POSTMASTER, PGC_S_ARGV);
+               break;
+           case 'k':
+               bootstrap_data_checksum_version = PG_DATA_CHECKSUM_VERSION;
+               break;
+           case 'r':
+               strlcpy(OutputFileName, optarg, MAXPGPATH);
+               break;
+           case 'x':
+               MyAuxProcType = atoi(optarg);
+               if (MyAuxProcType != CheckerProcess &&
+                   MyAuxProcType != BootstrapProcess)
+               {
+                   ereport(ERROR,
+                           (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                            errmsg("-x %s is invalid", optarg)));
+               }
+               break;
+           case 'X':
+               {
+                   int         WalSegSz = strtoul(optarg, NULL, 0);
+
+                   if (!IsValidWalSegSize(WalSegSz))
+                       ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("-X requires a power of two value between 1 MB and 1 GB")));
+                   SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL,
+                                   PGC_S_OVERRIDE);
+               }
+               break;
+           case 'c':
+           case '-':
+               {
+                   char       *name,
+                              *value;
+
+                   ParseLongOption(optarg, &name, &value);
+                   if (!value)
+                   {
+                       if (flag == '-')
+                           ereport(ERROR,
+                                   (errcode(ERRCODE_SYNTAX_ERROR),
+                                    errmsg("--%s requires a value",
+                                           optarg)));
+                       else
+                           ereport(ERROR,
+                                   (errcode(ERRCODE_SYNTAX_ERROR),
+                                    errmsg("-c %s requires a value",
+                                           optarg)));
+                   }
+
+                   SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
+                   free(name);
+                   if (value)
+                       free(value);
+                   break;
+               }
+           default:
+               write_stderr("Try \"%s --help\" for more information.\n",
+                            progname);
+               proc_exit(1);
+               break;
+       }
+   }
+
+   if (argc != optind)
+   {
+       write_stderr("%s: invalid command-line arguments\n", progname);
+       proc_exit(1);
+   }
+
+   /* Acquire configuration parameters */
+   if (!SelectConfigFiles(userDoption, progname))
+       proc_exit(1);
+
+   /*
+    * Validate we have been given a reasonable-looking DataDir and change
+    * into it
+    */
+   checkDataDir();
+   ChangeToDataDir();
+
+   CreateDataDirLockFile(false);
+
+   SetProcessingMode(BootstrapProcessing);
+   IgnoreSystemIndexes = true;
+
+   InitializeMaxBackends();
+
+   BaseInit();
+
+   /*
+    * XXX: It might make sense to move this into its own function at some
+    * point. Right now it seems like it'd cause more code duplication than
+    * it's worth.
+    */
+   if (MyAuxProcType == CheckerProcess)
+   {
+       SetProcessingMode(NormalProcessing);
+       CheckerModeMain();
+       abort();
+   }
+
+   bootstrap_signals();
+   BootStrapXLOG();
 
    /*
     * To ensure that src/common/link-canary.c is linked into the backend, we

diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index e58e24a6465e1ab45f8350198aef9cb7948f3647..78ec85861e93d30faac56c4c002a7b6be4c9f578 100644 (file)
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -198,7 +198,7 @@ main(int argc, char *argv[])
 #endif
 
    if (argc > 1 && strcmp(argv[1], "--boot") == 0)
-       AuxiliaryProcessMain(argc, argv);   /* does not return */
+       BootstrapModeMain(argc, argv);  /* does not return */
    else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0)
        GucInfoMain();          /* does not return */
    else if (argc > 1 && strcmp(argv[1], "--single") == 0)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a586c4175a39d809dfc9968c64a3c4572905e12b..fb948b725795bd18fd209ac68bb5a541c68129b5 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5026,6 +5026,10 @@ SubPostmasterMain(int argc, char *argv[])
    }
    if (strcmp(argv[1], "--forkaux") == 0)
    {
+       AuxProcType auxtype;
+
+       Assert(argc == 4);
+
        /* Restore basic shared memory pointers */
        InitShmemAccess(UsedShmemSegAddr);
 
@@ -5035,7 +5039,8 @@ SubPostmasterMain(int argc, char *argv[])
        /* Attach process to shared data structures */
        CreateSharedMemoryAndSemaphores();
 
-       AuxiliaryProcessMain(argc - 2, argv + 2);   /* does not return */
+       auxtype = atoi(argv[3]);
+       AuxiliaryProcessMain(auxtype);  /* does not return */
    }
    if (strcmp(argv[1], "--forkavlauncher") == 0)
    {
@@ -5414,28 +5419,28 @@ static pid_t
 StartChildProcess(AuxProcType type)
 {
    pid_t       pid;
-   char       *av[10];
-   int         ac = 0;
-   char        typebuf[32];
-
-   /*
-    * Set up command-line arguments for subprocess
-    */
-   av[ac++] = "postgres";
 
 #ifdef EXEC_BACKEND
-   av[ac++] = "--forkaux";
-   av[ac++] = NULL;            /* filled in by postmaster_forkexec */
-#endif
+   {
+       char       *av[10];
+       int         ac = 0;
+       char        typebuf[32];
 
-   snprintf(typebuf, sizeof(typebuf), "-x%d", type);
-   av[ac++] = typebuf;
+       /*
+        * Set up command-line arguments for subprocess
+        */
+       av[ac++] = "postgres";
+       av[ac++] = "--forkaux";
+       av[ac++] = NULL;        /* filled in by postmaster_forkexec */
 
-   av[ac] = NULL;
-   Assert(ac < lengthof(av));
+       snprintf(typebuf, sizeof(typebuf), "%d", type);
+       av[ac++] = typebuf;
 
-#ifdef EXEC_BACKEND
-   pid = postmaster_forkexec(ac, av);
+       av[ac] = NULL;
+       Assert(ac < lengthof(av));
+
+       pid = postmaster_forkexec(ac, av);
+   }
 #else                          /* !EXEC_BACKEND */
    pid = fork_process();
 
@@ -5451,7 +5456,7 @@ StartChildProcess(AuxProcType type)
        MemoryContextDelete(PostmasterContext);
        PostmasterContext = NULL;
 
-       AuxiliaryProcessMain(ac, av);   /* does not return */
+       AuxiliaryProcessMain(type); /* does not return */
    }
 #endif                         /* EXEC_BACKEND */
 

diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h
index 8290d4c6c490c32d90ee7cca18952e0bfe7649d4..0f8762afaf8099040962b2c752f900a90d2a00eb 100644 (file)
--- a/src/include/bootstrap/bootstrap.h
+++ b/src/include/bootstrap/bootstrap.h
@@ -15,6 +15,7 @@
 #define BOOTSTRAP_H
 
 #include "nodes/execnodes.h"
+#include "miscadmin.h"
 
 
 /*
@@ -32,7 +33,8 @@ extern Form_pg_attribute attrtypes[MAXATTR];
 extern int numattr;
 
 
-extern void AuxiliaryProcessMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern void BootstrapModeMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern void AuxiliaryProcessMain(AuxProcType auxtype) pg_attribute_noreturn();
 
 extern void closerel(char *name);
 extern void boot_openrel(char *name);