On Sat, Sep  1, 2012 at 02:35:06PM -0400, Bruce Momjian wrote:
> On Sat, Sep  1, 2012 at 02:23:22PM -0400, Tom Lane wrote:
> > Bruce Momjian <br...@momjian.us> writes:
> > > +         /*
> > > +          *      Report the Unix domain socket directory location to the 
> > > postmaster.
> > > +          */
> > 
> > "Report" seems entirely the wrong verb there.

Fixed.

> > > + #define LISTEN_STR      " -c listen_addresses=''"
> > > + 
> > > +         /* Have a sockdir to use? */
> > > +         if (strlen(os_info.sockdir) != 0)
> > > +                 snprintf(socket_string, sizeof(socket_string) - 
> > > strlen(LISTEN_STR),
> > > +                         " -c %s='%s'",
> > > +                         (GET_MAJOR_VERSION(cluster->major_version) < 
> > > 903) ?
> > > +                                 "unix_socket_directory" : 
> > > "unix_socket_directories",
> > > +                         os_info.sockdir);
> > > +         
> > > +         /* prevent TCP/IP connections */
> > > +         strcat(socket_string, LISTEN_STR);
> > 
> > IMO this would be simpler and more readable if you got rid of the LISTEN_STR
> > #define and just included -c listen_addresses='' in the snprintf format
> > string.  The comment for the whole thing should be something like
> > "If we have a socket directory to use, command the postmaster to use it,
> > and disable TCP/IP connections altogether".
> 
> Well, you only want the unix_socket* if sockdir is defined, but you want
> LISTEN_STR unconditionally, even if there is no sockdir.  Not sure how
> that could cleanly be in a single snprintf.

I restructured the code to add the listen_addresses string first,
allowing the removal of the #define, as Tom suggested.  I also added
unix_socket_permissions=0700 to further restrict socket access.

Updated patch attached.

-- 
  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 94bce50..8b35d8f
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*************** adjust_data_dir(ClusterInfo *cluster)
*** 376,378 ****
--- 376,431 ----
  
  	check_ok();
  }
+ 
+ 
+ #if HAVE_UNIX_SOCKETS
+ /*
+  * set_sockdir_and_pghost
+  *
+  * Set the socket directory to either the configured location (live check)
+  * or the current directory.
+  */
+ void
+ set_sockdir_and_pghost(bool live_check)
+ {
+ 	if (!live_check)
+ 	{
+ 		/* Use the current directory for the socket */
+ 		if (!getcwd(os_info.sockdir, MAXPGPATH))
+ 			pg_log(PG_FATAL, "cannot find current directory\n");
+ 	}
+ 	else
+ 	{
+ 		/*
+ 		 *	If we are doing a live check, we will use the old cluster's Unix
+ 		 *	domain socket directory so we can connect to the live server.
+ 		 */
+ 
+ 		/* sockdir added to the 5th line of postmaster.pid in PG 9.1 */
+ 		if (GET_MAJOR_VERSION(old_cluster.major_version) >= 901)
+ 		{
+ 			char		filename[MAXPGPATH];
+ 			FILE		*fp;
+ 			int			i;
+ 
+ 			snprintf(filename, sizeof(filename), "%s/postmaster.pid", old_cluster.pgdata);
+ 			if ((fp = fopen(filename, "r")) == NULL)
+ 				pg_log(PG_FATAL, "Could not get socket directory of the old server\n");
+ 
+ 			for (i = 0; i < 5; i++)
+ 				if (fgets(os_info.sockdir, sizeof(os_info.sockdir), fp) == NULL)
+ 					pg_log(PG_FATAL, "Could not get socket directory of the old server\n");
+ 			fclose(fp);
+ 		}
+ 		else
+ 			return;		/* Can't get live sockdir, so use the default. */
+ 
+ 		/* Remove trailing newline */
+ 		if (strchr(os_info.sockdir, '\n') != NULL)
+ 			*strchr(os_info.sockdir, '\n') = '\0';
+ 	}
+ 
+ 	/* For client communication */
+ 	pg_putenv("PGHOST", os_info.sockdir);
+ }
+ #endif
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index c47c8bb..8cad5c3
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*************** main(int argc, char **argv)
*** 88,93 ****
--- 88,97 ----
  	check_cluster_versions();
  	check_cluster_compatibility(live_check);
  
+ #if HAVE_UNIX_SOCKETS
+ 	set_sockdir_and_pghost(live_check);
+ #endif
+ 
  	check_old_cluster(live_check, &sequence_script_file_name);
  
  
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index fa4c6c0..a712e21
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 267,272 ****
--- 267,275 ----
  	const char *progname;		/* complete pathname for this program */
  	char	   *exec_path;		/* full path to my executable */
  	char	   *user;			/* username for clusters */
+ #if HAVE_UNIX_SOCKETS
+ 	char		sockdir[NAMEDATALEN];	/* directory for Unix Domain socket */
+ #endif
  	char	  **tablespaces;	/* tablespaces */
  	int			num_tablespaces;
  	char	  **libraries;		/* loadable libraries */
*************** void print_maps(FileNameMap *maps, int n
*** 387,392 ****
--- 390,398 ----
  
  void		parseCommandLine(int argc, char *argv[]);
  void		adjust_data_dir(ClusterInfo *cluster);
+ #if HAVE_UNIX_SOCKETS
+ void		set_sockdir_and_pghost(bool live_check);
+ #endif
  
  /* relfilenode.c */
  
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index 1fb0d6c..3d84f83
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*************** start_postmaster(ClusterInfo *cluster)
*** 144,149 ****
--- 144,150 ----
  	PGconn	   *conn;
  	bool		exit_hook_registered = false;
  	bool		pg_ctl_return = false;
+ 	char		socket_string[MAXPGPATH + 200];
  
  	if (!exit_hook_registered)
  	{
*************** start_postmaster(ClusterInfo *cluster)
*** 151,156 ****
--- 152,174 ----
  		exit_hook_registered = true;
  	}
  
+ 	socket_string[0] = '\0';
+ 
+ #ifdef HAVE_UNIX_SOCKETS
+ 	/* prevent TCP/IP connections, restrict socket access */
+ 	strcat(socket_string,
+ 		   " -c listen_addresses='' -c unix_socket_permissions=0700");
+ 
+ 	/* Have a sockdir?  Tell the postmaster. */
+ 	if (strlen(os_info.sockdir) != 0)
+ 		snprintf(socket_string + strlen(socket_string),
+ 				 sizeof(socket_string) - strlen(socket_string),
+ 				 " -c %s='%s'",
+ 				 (GET_MAJOR_VERSION(cluster->major_version) < 903) ?
+ 					"unix_socket_directory" : "unix_socket_directories",
+ 				 os_info.sockdir);
+ #endif
+ 
  	/*
  	 * Using autovacuum=off disables cleanup vacuum and analyze, but freeze
  	 * vacuums can still happen, so we set autovacuum_freeze_max_age to its
*************** start_postmaster(ClusterInfo *cluster)
*** 159,170 ****
  	 * not touch them.
  	 */
  	snprintf(cmd, sizeof(cmd),
! 			 "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d %s %s\" start",
  		  cluster->bindir, SERVER_LOG_FILE, cluster->pgconfig, cluster->port,
  			 (cluster->controldata.cat_ver >=
  			  BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
  			 "-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
! 			 cluster->pgopts ? cluster->pgopts : "");
  
  	/*
  	 * Don't throw an error right away, let connecting throw the error because
--- 177,188 ----
  	 * not touch them.
  	 */
  	snprintf(cmd, sizeof(cmd),
! 			 "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d %s %s%s\" start",
  		  cluster->bindir, SERVER_LOG_FILE, cluster->pgconfig, cluster->port,
  			 (cluster->controldata.cat_ver >=
  			  BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
  			 "-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
! 			 cluster->pgopts ? cluster->pgopts : "", socket_string);
  
  	/*
  	 * Don't throw an error right away, let connecting throw the error because
diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml
new file mode 100644
index b62aba2..b621911
*** a/doc/src/sgml/pgupgrade.sgml
--- b/doc/src/sgml/pgupgrade.sgml
*************** psql --username postgres --file script.s
*** 520,525 ****
--- 520,532 ----
    </para>
  
    <para>
+    If running check on an old pre-9.1 Unix-like running server, and the
+    old and new servers use different Unix-domain socket directories,
+    use the <option>-O</> option so the new server uses the same socket
+    directory as the old server, and set <envar>PGHOST</> similarly.
+   </para>
+ 
+   <para>
     A Log-Shipping Standby Server (<xref linkend="warm-standby">) cannot
     be upgraded because the server must allow writes.  The simplest way
     is to upgrade the primary and use rsync to rebuild the standbys.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to