Re: Bug with pg_ctl -w/wait and config-only directories - Mailing list pgsql-hackers

Bruce Momjian wrote:
> Robert Haas wrote:
> > On Mon, Oct 3, 2011 at 11:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
> > > OK, here is a patch that adds a -C option to the postmaster so any
> > > config variable can be dumped, even while the server is running (there
> > > is no security check because we don't have a user name at this point),

OK, here is an updated patch, with documentation, that I consider ready
for commit to git head.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
new file mode 100644
index b16bbdf..3807f40
*** a/doc/src/sgml/ref/postgres-ref.sgml
--- b/doc/src/sgml/ref/postgres-ref.sgml
*************** PostgreSQL documentation
*** 141,146 ****
--- 141,160 ----
       </varlistentry>

       <varlistentry>
+       <term><option>-C <replaceable>name</replaceable></option></term>
+       <listitem>
+        <para>
+         Returns the value of a named run-time parameter, and exits.
+         (See the <option>-c</> option above for details.)  This can
+         be used on a running server, and returns values from
+         <filename>postgresql.conf</>, modified by any parameters
+         supplied in this invocation.  It does not reflect parameters
+         supplied when the cluster was started.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term><option>-d <replaceable>debug-level</replaceable></option></term>
        <listitem>
         <para>
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
new file mode 100644
index 0a84d97..dd7493c
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
*************** bool        enable_bonjour = false;
*** 203,208 ****
--- 203,210 ----
  char       *bonjour_name;
  bool        restart_after_crash = true;

+ char         *output_config_variable = NULL;
+
  /* PIDs of special child processes; 0 when not running */
  static pid_t StartupPID = 0,
              BgWriterPID = 0,
*************** PostmasterMain(int argc, char *argv[])
*** 537,543 ****
       * tcop/postgres.c (the option sets should not conflict) and with the
       * common help() function in main/main.c.
       */
!     while ((opt = getopt(argc, argv, "A:B:bc:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
      {
          switch (opt)
          {
--- 539,545 ----
       * tcop/postgres.c (the option sets should not conflict) and with the
       * common help() function in main/main.c.
       */
!     while ((opt = getopt(argc, argv, "A:B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
      {
          switch (opt)
          {
*************** PostmasterMain(int argc, char *argv[])
*** 554,559 ****
--- 556,565 ----
                  IsBinaryUpgrade = true;
                  break;

+             case 'C':
+                 output_config_variable = optarg;
+                 break;
+
              case 'D':
                  userDoption = optarg;
                  break;
*************** PostmasterMain(int argc, char *argv[])
*** 728,733 ****
--- 734,746 ----
      if (!SelectConfigFiles(userDoption, progname))
          ExitPostmaster(2);

+     if (output_config_variable != NULL)
+     {
+         /* permission is handled because the user is reading inside the data dir */
+         puts(GetConfigOption(output_config_variable, false, false));
+         ExitPostmaster(0);
+     }
+
      /* Verify that DataDir looks reasonable */
      checkDataDir();

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
new file mode 100644
index c7eac71..19d94b2
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** process_postgres_switches(int argc, char
*** 3170,3176 ****
       * postmaster/postmaster.c (the option sets should not conflict) and with
       * the common help() function in main/main.c.
       */
!     while ((flag = getopt(argc, argv, "A:B:bc:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1)
      {
          switch (flag)
          {
--- 3170,3176 ----
       * postmaster/postmaster.c (the option sets should not conflict) and with
       * the common help() function in main/main.c.
       */
!     while ((flag = getopt(argc, argv, "A:B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1)
      {
          switch (flag)
          {
*************** process_postgres_switches(int argc, char
*** 3187,3192 ****
--- 3187,3196 ----
                  IsBinaryUpgrade = true;
                  break;

+             case 'C':
+                 /* ignored for consistency with the postmaster */
+                 break;
+
              case 'D':
                  if (secure)
                      userDoption = strdup(optarg);
*************** process_postgres_switches(int argc, char
*** 3272,3278 ****
                  break;

              case 'T':
!                 /* ignored for consistency with postmaster */
                  break;

              case 't':
--- 3276,3282 ----
                  break;

              case 'T':
!                 /* ignored for consistency with the postmaster */
                  break;

              case 't':
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
new file mode 100644
index 0dbdfe7..e633d0c
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
*************** static ShutdownMode shutdown_mode = SMAR
*** 81,86 ****
--- 81,87 ----
  static int    sig = SIGTERM;        /* default */
  static CtlCommand ctl_command = NO_COMMAND;
  static char *pg_data = NULL;
+ static char *pg_config = NULL;
  static char *pgdata_opt = NULL;
  static char *post_opts = NULL;
  static const char *progname;
*************** static void do_status(void);
*** 131,136 ****
--- 132,138 ----
  static void do_promote(void);
  static void do_kill(pgpid_t pid);
  static void print_msg(const char *msg);
+ static void adjust_data_dir(void);

  #if defined(WIN32) || defined(__CYGWIN__)
  static bool pgwin32_IsInstalled(SC_HANDLE);
*************** pgwin32_CommandLine(bool registration)
*** 1265,1274 ****
          strcat(cmdLine, "\"");
      }

!     if (pg_data)
      {
          strcat(cmdLine, " -D \"");
!         strcat(cmdLine, pg_data);
          strcat(cmdLine, "\"");
      }

--- 1267,1276 ----
          strcat(cmdLine, "\"");
      }

!     if (pg_config)
      {
          strcat(cmdLine, " -D \"");
!         strcat(cmdLine, pg_config);
          strcat(cmdLine, "\"");
      }

*************** set_starttype(char *starttypeopt)
*** 1886,1891 ****
--- 1888,1946 ----
  }
  #endif

+ /*
+  * adjust_data_dir
+  *
+  * If a configuration-only directory was specified, find the real data dir.
+  */
+ void
+ adjust_data_dir(void)
+ {
+     char        cmd[MAXPGPATH], filename[MAXPGPATH], *my_exec_path;
+     FILE       *fd;
+
+     /* If there is no postgresql.conf, it can't be a config-only dir */
+     snprintf(filename, sizeof(filename), "%s/postgresql.conf", pg_config);
+     if ((fd = fopen(filename, "r")) == NULL)
+         return;
+     fclose(fd);
+
+     /* If PG_VERSION exists, it can't be a config-only dir */
+     snprintf(filename, sizeof(filename), "%s/PG_VERSION", pg_config);
+     if ((fd = fopen(filename, "r")) != NULL)
+     {
+         fclose(fd);
+         return;
+     }
+
+     /* Must be a configuration directory, so find the data directory */
+
+     /* we use a private my_exec_path to avoid interfering with later uses */
+     if (exec_path == NULL)
+         my_exec_path = find_other_exec_or_die(argv0, "postgres", PG_BACKEND_VERSIONSTR);
+     else
+         my_exec_path = xstrdup(exec_path);
+
+     snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s -C data_directory" SYSTEMQUOTE,
+              my_exec_path, pgdata_opt ? pgdata_opt : "", post_opts ?
+              post_opts : "");
+
+     fd = popen(cmd, "r");
+     if (fd == NULL || fgets(filename, sizeof(filename), fd) == NULL)
+     {
+         write_stderr(_("%s: cannot find the data directory using %s\n"), progname, my_exec_path);
+         exit(1);
+     }
+     pclose(fd);
+     free(my_exec_path);
+
+     if (strlen(filename) > 0 && filename[strlen(filename) - 1] == '\n')
+         filename[strlen(filename) - 1] = '\0';
+     free(pg_data);
+     pg_data = xstrdup(filename);
+     canonicalize_path(pg_data);
+ }
+

  int
  main(int argc, char **argv)
*************** main(int argc, char **argv)
*** 2120,2133 ****
      }

      /* Note we put any -D switch into the env var above */
!     pg_data = getenv("PGDATA");
!     if (pg_data)
      {
!         pg_data = xstrdup(pg_data);
!         canonicalize_path(pg_data);
      }

!     if (pg_data == NULL &&
          ctl_command != KILL_COMMAND && ctl_command != UNREGISTER_COMMAND)
      {
          write_stderr(_("%s: no database directory specified and environment variable PGDATA unset\n"),
--- 2175,2191 ----
      }

      /* Note we put any -D switch into the env var above */
!     pg_config = getenv("PGDATA");
!     if (pg_config)
      {
!         pg_config = xstrdup(pg_config);
!         canonicalize_path(pg_config);
!         pg_data = xstrdup(pg_config);
      }

!     adjust_data_dir();
!
!     if (pg_config == NULL &&
          ctl_command != KILL_COMMAND && ctl_command != UNREGISTER_COMMAND)
      {
          write_stderr(_("%s: no database directory specified and environment variable PGDATA unset\n"),