Re: Testing 9.0beta3 and pg_upgrade

Thomas Kellerer wrote:
> Thomas Kellerer, 12.07.2010 23:29:
> > Hi,
> >
> > I'm trying pg_upgrade on my Windows installation and I have two
> > suggestions for the manual regarding pg_upgrade:
> >
> > When specifying directories, pg_upgrade *requires* a forward slash as
> > the path separator. This is (still) uncommon in the Windows world
> > (although Windows does support it) and even though the example in the
> > manual does show forward slashes, I think it would be a good idea to
> > specifically mention the fact that it will *not* work with a backslash.
>
> There is another misleading error message.
>
> When the old *bin*dir is not specified correctly, pg_upgrade claims the old *data*dir does not exist
>
> Something like:
>
> pg_upgrade --check --old-bindir="c:/Program Files/PostgreSQL/8.4"  ....
>
> (note the missing /bin part)
>
> will cause the following output:
>
> 'c:/Program' is not recognized as an internal or external command,
> operable program or batch file.
> Performing Consistency Checks
> -----------------------------
> Checking old data directory (c:/Daten/db/pgdata84)
> check for postgres failed - No such file or directory
>
> It took me a while to find out that the bindir was wrong, not the datadir. The "c:/Program' is not recognized as
an.."made it even more confusing. 

OK, I have modified pg_upgrade with the attachd patch that reports /bin
and /data directory tests separately.  Thanks for the report.

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

  + None of us is going to be here forever. +
Index: contrib/pg_upgrade/exec.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/exec.c,v
retrieving revision 1.8
diff -c -c -r1.8 exec.c
*** contrib/pg_upgrade/exec.c    6 Jul 2010 19:18:55 -0000    1.8
--- contrib/pg_upgrade/exec.c    13 Jul 2010 17:59:33 -0000
***************
*** 13,22 ****
  #include <grp.h>


! static void checkBinDir(migratorContext *ctx, ClusterInfo *cluster);
  static int    check_exec(migratorContext *ctx, const char *dir, const char *cmdName);
  static const char *validate_exec(const char *path);
- static int    check_data_dir(migratorContext *ctx, const char *pg_data);


  /*
--- 13,22 ----
  #include <grp.h>


! static void    check_data_dir(migratorContext *ctx, const char *pg_data);
! static void check_bin_dir(migratorContext *ctx, ClusterInfo *cluster);
  static int    check_exec(migratorContext *ctx, const char *dir, const char *cmdName);
  static const char *validate_exec(const char *path);


  /*
***************
*** 56,61 ****
--- 56,89 ----


  /*
+  * is_server_running()
+  *
+  * checks whether postmaster on the given data directory is running or not.
+  * The check is performed by looking for the existence of postmaster.pid file.
+  */
+ bool
+ is_server_running(migratorContext *ctx, const char *datadir)
+ {
+     char        path[MAXPGPATH];
+     int            fd;
+
+     snprintf(path, sizeof(path), "%s/postmaster.pid", datadir);
+
+     if ((fd = open(path, O_RDONLY, 0)) < 0)
+     {
+         if (errno != ENOENT)
+             pg_log(ctx, PG_FATAL, "\ncould not open file \"%s\" for reading\n",
+                    path);
+
+         return false;
+     }
+
+     close(fd);
+     return true;
+ }
+
+
+ /*
   * verify_directories()
   *
   * does all the hectic work of verifying directories and executables
***************
*** 67,87 ****
  verify_directories(migratorContext *ctx)
  {
      prep_status(ctx, "Checking old data directory (%s)", ctx->old.pgdata);
!     if (check_data_dir(ctx, ctx->old.pgdata) != 0)
!         pg_log(ctx, PG_FATAL, "Failed\n");
!     checkBinDir(ctx, &ctx->old);
      check_ok(ctx);

      prep_status(ctx, "Checking new data directory (%s)", ctx->new.pgdata);
!     if (check_data_dir(ctx, ctx->new.pgdata) != 0)
!         pg_log(ctx, PG_FATAL, "Failed\n");
!     checkBinDir(ctx, &ctx->new);
      check_ok(ctx);
  }


  /*
!  * checkBinDir()
   *
   *    This function searches for the executables that we expect to find
   *    in the binaries directory.    If we find that a required executable
--- 95,156 ----
  verify_directories(migratorContext *ctx)
  {
      prep_status(ctx, "Checking old data directory (%s)", ctx->old.pgdata);
!     check_data_dir(ctx, ctx->old.pgdata);
!     check_ok(ctx);
!
!     prep_status(ctx, "Checking old bin directory (%s)", ctx->old.bindir);
!     check_bin_dir(ctx, &ctx->old);
      check_ok(ctx);

      prep_status(ctx, "Checking new data directory (%s)", ctx->new.pgdata);
!     check_data_dir(ctx, ctx->new.pgdata);
!     check_ok(ctx);
!
!     prep_status(ctx, "Checking new bin directory (%s)", ctx->new.bindir);
!     check_bin_dir(ctx, &ctx->new);
      check_ok(ctx);
  }


  /*
!  * check_data_dir()
!  *
!  *    This function validates the given cluster directory - we search for a
!  *    small set of subdirectories that we expect to find in a valid $PGDATA
!  *    directory.    If any of the subdirectories are missing (or secured against
!  *    us) we display an error message and exit()
!  *
!  */
! static void
! check_data_dir(migratorContext *ctx, const char *pg_data)
! {
!     char        subDirName[MAXPGPATH];
!     int            subdirnum;
!     const char *requiredSubdirs[] = {"base", "global", "pg_clog",
!         "pg_multixact", "pg_subtrans", "pg_tblspc", "pg_twophase",
!         "pg_xlog"};
!
!     for (subdirnum = 0;
!          subdirnum < sizeof(requiredSubdirs) / sizeof(requiredSubdirs[0]);
!          ++subdirnum)
!     {
!         struct stat statBuf;
!
!         snprintf(subDirName, sizeof(subDirName), "%s/%s", pg_data,
!                  requiredSubdirs[subdirnum]);
!
!         if (stat(subDirName, &statBuf) != 0)
!             report_status(ctx, PG_FATAL, "check for %s failed:  %s",
!                           requiredSubdirs[subdirnum], getErrorText(errno));
!         else if (!S_ISDIR(statBuf.st_mode))
!                 report_status(ctx, PG_FATAL, "%s is not a directory",
!                               requiredSubdirs[subdirnum]);
!     }
! }
!
!
! /*
!  * check_bin_dir()
   *
   *    This function searches for the executables that we expect to find
   *    in the binaries directory.    If we find that a required executable
***************
*** 89,95 ****
   *    exit().
   */
  static void
! checkBinDir(migratorContext *ctx, ClusterInfo *cluster)
  {
      check_exec(ctx, cluster->bindir, "postgres");
      check_exec(ctx, cluster->bindir, "psql");
--- 158,164 ----
   *    exit().
   */
  static void
! check_bin_dir(migratorContext *ctx, ClusterInfo *cluster)
  {
      check_exec(ctx, cluster->bindir, "postgres");
      check_exec(ctx, cluster->bindir, "psql");
***************
*** 99,132 ****


  /*
-  * is_server_running()
-  *
-  * checks whether postmaster on the given data directory is running or not.
-  * The check is performed by looking for the existence of postmaster.pid file.
-  */
- bool
- is_server_running(migratorContext *ctx, const char *datadir)
- {
-     char        path[MAXPGPATH];
-     int            fd;
-
-     snprintf(path, sizeof(path), "%s/postmaster.pid", datadir);
-
-     if ((fd = open(path, O_RDONLY, 0)) < 0)
-     {
-         if (errno != ENOENT)
-             pg_log(ctx, PG_FATAL, "\ncould not open file \"%s\" for reading\n",
-                    path);
-
-         return false;
-     }
-
-     close(fd);
-     return true;
- }
-
-
- /*
   * check_exec()
   *
   *    Checks whether either of the two command names (cmdName and alternative)
--- 168,173 ----
***************
*** 264,313 ****
      return NULL;
  #endif
  }
-
-
- /*
-  * check_data_dir()
-  *
-  *    This function validates the given cluster directory - we search for a
-  *    small set of subdirectories that we expect to find in a valid $PGDATA
-  *    directory.    If any of the subdirectories are missing (or secured against
-  *    us) we display an error message and exit()
-  *
-  */
- static int
- check_data_dir(migratorContext *ctx, const char *pg_data)
- {
-     char        subDirName[MAXPGPATH];
-     const char *requiredSubdirs[] = {"base", "global", "pg_clog",
-         "pg_multixact", "pg_subtrans",
-     "pg_tblspc", "pg_twophase", "pg_xlog"};
-     bool        fail = false;
-     int            subdirnum;
-
-     for (subdirnum = 0; subdirnum < sizeof(requiredSubdirs) / sizeof(requiredSubdirs[0]); ++subdirnum)
-     {
-         struct stat statBuf;
-
-         snprintf(subDirName, sizeof(subDirName), "%s/%s", pg_data,
-                  requiredSubdirs[subdirnum]);
-
-         if ((stat(subDirName, &statBuf)) != 0)
-         {
-             report_status(ctx, PG_WARNING, "check for %s warning:  %s",
-                           requiredSubdirs[subdirnum], getErrorText(errno));
-             fail = true;
-         }
-         else
-         {
-             if (!S_ISDIR(statBuf.st_mode))
-             {
-                 report_status(ctx, PG_WARNING, "%s is not a directory",
-                               requiredSubdirs[subdirnum]);
-                 fail = true;
-             }
-         }
-     }
-
-     return (fail) ? -1 : 0;
- }
--- 305,307 ----