Re: pg_upgrade - add config directory setting - Mailing list pgsql-hackers

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Tom Lane wrote:
> > >> Yeah.  I think the only sensible way to do this would be to provide an
> > >> operating mode for the postgres executable that would just parse the
> > >> config file and spit out requested values.
> >
> > > That would certainly solve the problem, though it would have to be
> > > backpatched all the way back to 8.4, and it would require pg_upgrade
> > > users to be on newer minor versions of Postgres.
> >
> > I would just say "no" to people who expect this to work against older
> > versions of Postgres.  I think it's sufficient if we get this into HEAD
> > so that it will work in the future.
>
> Well, it is going to work in the future only when the _old_ version is
> 9.2+.  Specifically, pg_upgrade using the flag could be patched to just
> 9.2, but the flag has to be supported on old and new backends for that
> to work.

OK, I started working on #3, which was to start the servers to find the
data_directory setting, and developed the attached patch which mostly
does this.  However, I have found serious problems with pg_ctl -w/wait
mode and config-only directories (which pg_upgrade uses), and will start
a new thread to address this issue and then continue with this once that
is resolved.

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

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index bdb7ddb..e9c5f25
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*************** parseCommandLine(int argc, char *argv[])
*** 112,121 ****
--- 112,123 ----

              case 'd':
                  old_cluster.pgdata = pg_strdup(optarg);
+                 old_cluster.pgconfig = pg_strdup(optarg);
                  break;

              case 'D':
                  new_cluster.pgdata = pg_strdup(optarg);
+                 new_cluster.pgconfig = pg_strdup(optarg);
                  break;

              case 'g':
*************** check_required_directory(char **dirpath,
*** 319,321 ****
--- 321,379 ----
  #endif
          (*dirpath)[strlen(*dirpath) - 1] = 0;
  }
+
+ /*
+  * adjust_data_dir
+  *
+  * If a configuration-only directory was specified, find the real data dir
+  * by quering the running server.  This has limited checking because we
+  * can't check for a running server because we can't find postmaster.pid.
+  */
+ void
+ adjust_data_dir(ClusterInfo *cluster)
+ {
+     char        filename[MAXPGPATH];
+     FILE       *fd;
+     PGconn       *conn;
+     PGresult   *res;
+
+     /* If there is no postgresql.conf, it can't be a config-only dir */
+     snprintf(filename, sizeof(filename), "%s/postgresql.conf", cluster->pgconfig);
+     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", cluster->pgconfig);
+     if ((fd = fopen(filename, "r")) != NULL)
+     {
+         fclose(fd);
+         return;
+     }
+
+     /* Must be a configuration directory, so query the server. */
+
+     prep_status("Finding the real data directory for the %s cluster",
+                 CLUSTER_NAME(cluster));
+
+     start_postmaster(cluster);
+
+     conn = connectToServer(cluster, "template1");
+
+     res = executeQueryOrDie(conn,
+                             "SELECT setting "
+                             "FROM pg_settings "
+                             "WHERE name = 'data_directory'");
+     assert(PQntuples(res) == 1);
+
+     pg_free(cluster->pgdata);
+     cluster->pgdata = pg_strdup(PQgetvalue(res, 0, 0));
+
+     PQclear(res);
+
+     PQfinish(conn);
+
+     stop_postmaster(false);
+
+     check_ok();
+ }
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index 0568aca..273561e
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*************** main(int argc, char **argv)
*** 68,73 ****
--- 68,76 ----

      parseCommandLine(argc, argv);

+     adjust_data_dir(&old_cluster);
+     adjust_data_dir(&new_cluster);
+
      output_check_banner(&live_check);

      setup(argv[0], live_check);
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index 46aed74..0fb16ed
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 187,192 ****
--- 187,193 ----
      ControlData controldata;    /* pg_control information */
      DbInfoArr    dbarr;            /* dbinfos array */
      char       *pgdata;            /* pathname for cluster's $PGDATA directory */
+     char       *pgconfig;        /* pathname for cluster's config file directory */
      char       *bindir;            /* pathname for cluster's executable directory */
      unsigned short port;        /* port number where postmaster is waiting */
      uint32        major_version;    /* PG_VERSION of cluster */
*************** void print_maps(FileNameMap *maps, int n
*** 361,366 ****
--- 362,368 ----
  /* option.c */

  void        parseCommandLine(int argc, char *argv[]);
+ void        adjust_data_dir(ClusterInfo *cluster);

  /* relfilenode.c */

diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index 8c4aec9..d12ff64
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*************** start_postmaster(ClusterInfo *cluster)
*** 164,175 ****
       * vacuums can still happen, so we set autovacuum_freeze_max_age to its
       * maximum.  We assume all datfrozenxid and relfrozen values are less than
       * a gap of 2000000000 from the current xid counter, so autovacuum will
!      * not touch them.
       */
      snprintf(cmd, sizeof(cmd),
               SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" "
               "-o \"-p %d %s\" start >> \"%s\" 2>&1" SYSTEMQUOTE,
!              cluster->bindir, log_opts.filename2, cluster->pgdata, cluster->port,
               (cluster->controldata.cat_ver >=
                BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
               "-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
--- 164,176 ----
       * vacuums can still happen, so we set autovacuum_freeze_max_age to its
       * maximum.  We assume all datfrozenxid and relfrozen values are less than
       * a gap of 2000000000 from the current xid counter, so autovacuum will
!      * not touch them.  When we are starting the server to find the real data
!      * directory, controldata.cat_ver is zero, meaning we assume an older server.
       */
      snprintf(cmd, sizeof(cmd),
               SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" "
               "-o \"-p %d %s\" start >> \"%s\" 2>&1" SYSTEMQUOTE,
!              cluster->bindir, log_opts.filename2, cluster->pgconfig, cluster->port,
               (cluster->controldata.cat_ver >=
                BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
               "-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
*************** stop_postmaster(bool fast)
*** 210,215 ****
--- 211,221 ----
      const char *bindir;
      const char *datadir;

+     /*
+      * Oddly, for config-only directories, pg_ctl _stop_ needs the real
+      * data directory to check postmasteer.pid, not the config directory.
+      */
+
      if (os_info.running_cluster == &old_cluster)
      {
          bindir = old_cluster.bindir;
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index a18f14a..b2f8239
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 46,52 ****
                      # (change requires restart)

  # If external_pid_file is not explicitly set, no extra PID file is written.
! #external_pid_file = '(none)'        # write an extra PID file
                      # (change requires restart)


--- 46,52 ----
                      # (change requires restart)

  # If external_pid_file is not explicitly set, no extra PID file is written.
! #external_pid_file = ''        # write an extra PID file
                      # (change requires restart)