On Thu, Jul 05, 2012 at 06:28:31PM -0400, Tom Lane wrote:
> Payal Singh <paya...@umbc.edu> writes:
> > On Thu, Jul 5, 2012 at 12:15 PM, Craig Ringer <ring...@ringerc.id.au> wrote:
> >> If you start 9.1 on a copy of the backup then cleanly stop it again, does
> >> pg_upgrade then run?
> 
> > Thank you. That worked.
> 
> ISTM that pg_upgrade should check that the old cluster was shut down
> cleanly, ie pg_control has state = "shut down".  AFAICT from some
> testing, it currently only checks that there is no postmaster.pid file,
> which is easily bypassed by users who might not realize that it's not
> safe to run pg_upgrade against a filesystem backup.

I am confused.  pg_upgrade certainly starts/stops the old and new server
with pg_ctl before copying any files --- isn't that sufficent?

> BTW, I also noticed while trying to test this that pg_upgrade is
> currently completely broken for the case of taking PGDATAOLD or
> PGDATANEW from the environment rather than switches.  This is because
> the existing coding in option.c fails to set up the "pgconfig" fields
> in such cases.

Oh, good catch.  Fixed with the attached patch, and backpatched to 9.2.

-- 
  Bruce Momjian  <br...@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 43394a0..960fcda
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
***************
*** 21,27 ****
  
  
  static void usage(void);
! static void check_required_directory(char **dirpath,
  				   char *envVarName, char *cmdLineOption, char *description);
  
  
--- 21,27 ----
  
  
  static void usage(void);
! static void check_required_directory(char **dirpath, char **configpath,
  				   char *envVarName, char *cmdLineOption, char *description);
  
  
*************** parseCommandLine(int argc, char *argv[])
*** 203,216 ****
  	}
  
  	/* Get values from env if not already set */
! 	check_required_directory(&old_cluster.bindir, "PGBINOLD", "-b",
  							 "old cluster binaries reside");
! 	check_required_directory(&new_cluster.bindir, "PGBINNEW", "-B",
  							 "new cluster binaries reside");
! 	check_required_directory(&old_cluster.pgdata, "PGDATAOLD", "-d",
! 							 "old cluster data resides");
! 	check_required_directory(&new_cluster.pgdata, "PGDATANEW", "-D",
! 							 "new cluster data resides");
  }
  
  
--- 203,216 ----
  	}
  
  	/* Get values from env if not already set */
! 	check_required_directory(&old_cluster.bindir, NULL, "PGBINOLD", "-b",
  							 "old cluster binaries reside");
! 	check_required_directory(&new_cluster.bindir, NULL, "PGBINNEW", "-B",
  							 "new cluster binaries reside");
! 	check_required_directory(&old_cluster.pgdata, &old_cluster.pgconfig,
! 							 "PGDATAOLD", "-d", "old cluster data resides");
! 	check_required_directory(&new_cluster.pgdata, &new_cluster.pgconfig,
! 							 "PGDATANEW", "-D", "new cluster data resides");
  }
  
  
*************** or\n"), old_cluster.port, new_cluster.po
*** 284,298 ****
   * user hasn't provided the required directory name.
   */
  static void
! check_required_directory(char **dirpath, char *envVarName,
! 						 char *cmdLineOption, char *description)
  {
  	if (*dirpath == NULL || strlen(*dirpath) == 0)
  	{
  		const char *envVar;
  
  		if ((envVar = getenv(envVarName)) && strlen(envVar))
  			*dirpath = pg_strdup(envVar);
  		else
  			pg_log(PG_FATAL, "You must identify the directory where the %s.\n"
  				   "Please use the %s command-line option or the %s environment variable.\n",
--- 284,303 ----
   * user hasn't provided the required directory name.
   */
  static void
! check_required_directory(char **dirpath, char **configpath,
! 						 char *envVarName, char *cmdLineOption,
! 						 char *description)
  {
  	if (*dirpath == NULL || strlen(*dirpath) == 0)
  	{
  		const char *envVar;
  
  		if ((envVar = getenv(envVarName)) && strlen(envVar))
+ 		{
  			*dirpath = pg_strdup(envVar);
+ 			if (configpath)
+ 				*configpath = pg_strdup(envVar);
+ 		}
  		else
  			pg_log(PG_FATAL, "You must identify the directory where the %s.\n"
  				   "Please use the %s command-line option or the %s environment variable.\n",
-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to