From 2d34ad84303181111c6f0747186857ff50106267 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 1 Dec 2018 15:45:11 -0500 Subject: [PATCH] Add a --socketdir option to pg_upgrade. This allows control of the directory in which the postmaster sockets are created for the temporary postmasters started by pg_upgrade. The default location remains the current working directory, which is typically fine, but if it is deeply nested then its pathname might be too long to be a socket name. In passing, clean up some messiness in pg_upgrade's option handling, particularly the confusing and undocumented way that configuration-only datadirs were handled. And fix check_required_directory's substantially under-baked cleanup of directory pathnames. Daniel Gustafsson, reviewed by Hironobu Suzuki, some code cleanup by me Discussion: https://postgr.es/m/E72DD5C3-2268-48A5-A907-ED4B34BEC223@yesql.se --- doc/src/sgml/ref/pgupgrade.sgml | 36 ++++++++++++--- src/bin/pg_upgrade/option.c | 80 ++++++++++++++++++--------------- src/bin/pg_upgrade/pg_upgrade.h | 5 ++- 3 files changed, 77 insertions(+), 44 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 978fa252e4..7e1afaf0a5 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -163,6 +163,14 @@ + + dir + dir + directory to use for postmaster sockets during upgrade; + default is current working directory; environment + variable PGSOCKETDIR + + username username @@ -709,11 +717,21 @@ psql --username=postgres --file=script.sql postgres Notes - pg_upgrade does not support upgrading of databases - containing table columns using these reg* OID-referencing system data types: - regproc, regprocedure, regoper, - regoperator, regconfig, and - regdictionary. (regtype can be upgraded.) + pg_upgrade creates various working files, such + as schema dumps, in the current working directory. For security, be sure + that that directory is not readable or writable by any other users. + + + + pg_upgrade launches short-lived postmasters in + the old and new data directories. Temporary Unix socket files for + communication with these postmasters are, by default, made in the current + working directory. In some situations the path name for the current + directory might be too long to be a valid socket name. In that case you + can use the option to put the socket files in some + directory with a shorter path name. For security, be sure that that + directory is not readable or writable by any other users. + (This is not relevant on Windows.) @@ -732,6 +750,14 @@ psql --username=postgres --file=script.sql postgres insert dummy data, and upgrade that. + + pg_upgrade does not support upgrading of databases + containing table columns using these reg* OID-referencing system data types: + regproc, regprocedure, regoper, + regoperator, regconfig, and + regdictionary. (regtype can be upgraded.) + + If you are upgrading a pre-PostgreSQL 9.2 cluster that uses a configuration-file-only directory, you must pass the diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index 7cc92f2dcb..ef8dd28adb 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -21,8 +21,9 @@ static void usage(void); -static void check_required_directory(char **dirpath, char **configpath, - const char *envVarName, const char *cmdLineOption, const char *description); +static void check_required_directory(char **dirpath, + const char *envVarName, bool useCwd, + const char *cmdLineOption, const char *description); #define FIX_DEFAULT_READ_ONLY "-c default_transaction_read_only=false" @@ -52,6 +53,7 @@ parseCommandLine(int argc, char *argv[]) {"link", no_argument, NULL, 'k'}, {"retain", no_argument, NULL, 'r'}, {"jobs", required_argument, NULL, 'j'}, + {"socketdir", required_argument, NULL, 's'}, {"verbose", no_argument, NULL, 'v'}, {"clone", no_argument, NULL, 1}, @@ -102,7 +104,7 @@ parseCommandLine(int argc, char *argv[]) if ((log_opts.internal = fopen_priv(INTERNAL_LOG_FILE, "a")) == NULL) pg_fatal("could not write to log file \"%s\"\n", INTERNAL_LOG_FILE); - while ((option = getopt_long(argc, argv, "d:D:b:B:cj:ko:O:p:P:rU:v", + while ((option = getopt_long(argc, argv, "d:D:b:B:cj:ko:O:p:P:rs:U:v", long_options, &optindex)) != -1) { switch (option) @@ -121,12 +123,10 @@ parseCommandLine(int argc, char *argv[]) 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 'j': @@ -188,6 +188,10 @@ parseCommandLine(int argc, char *argv[]) log_opts.retain = true; break; + case 's': + user_opts.socketdir = pg_strdup(optarg); + break; + case 'U': pg_free(os_info.user); os_info.user = pg_strdup(optarg); @@ -244,14 +248,16 @@ parseCommandLine(int argc, char *argv[]) pg_putenv("PGOPTIONS", FIX_DEFAULT_READ_ONLY); /* 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")); + check_required_directory(&old_cluster.bindir, "PGBINOLD", false, + "-b", _("old cluster binaries reside")); + check_required_directory(&new_cluster.bindir, "PGBINNEW", false, + "-B", _("new cluster binaries reside")); + check_required_directory(&old_cluster.pgdata, "PGDATAOLD", false, + "-d", _("old cluster data resides")); + check_required_directory(&new_cluster.pgdata, "PGDATANEW", false, + "-D", _("new cluster data resides")); + check_required_directory(&user_opts.socketdir, "PGSOCKETDIR", true, + "-s", _("sockets will be created")); #ifdef WIN32 @@ -296,6 +302,7 @@ usage(void) printf(_(" -p, --old-port=PORT old cluster port number (default %d)\n"), old_cluster.port); printf(_(" -P, --new-port=PORT new cluster port number (default %d)\n"), new_cluster.port); printf(_(" -r, --retain retain SQL and log files after success\n")); + printf(_(" -s, --socketdir=DIR socket directory to use (default CWD)\n")); printf(_(" -U, --username=NAME cluster superuser (default \"%s\")\n"), os_info.user); printf(_(" -v, --verbose enable verbose internal logging\n")); printf(_(" -V, --version display version information, then exit\n")); @@ -337,29 +344,32 @@ usage(void) * check_required_directory() * * Checks a directory option. - * dirpath - the directory name supplied on the command line - * configpath - optional configuration directory + * dirpath - the directory name supplied on the command line, or NULL * envVarName - the name of an environment variable to get if dirpath is NULL - * cmdLineOption - the command line option corresponds to this directory (-o, -O, -n, -N) + * useCwd - true if OK to default to CWD + * cmdLineOption - the command line option for this directory * description - a description of this directory option * * We use the last two arguments to construct a meaningful error message if the * user hasn't provided the required directory name. */ static void -check_required_directory(char **dirpath, char **configpath, - const char *envVarName, const char *cmdLineOption, - const char *description) +check_required_directory(char **dirpath, const char *envVarName, bool useCwd, + const char *cmdLineOption, const 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 if (useCwd) + { + char cwd[MAXPGPATH]; + + if (!getcwd(cwd, MAXPGPATH)) + pg_fatal("could not determine current directory\n"); + *dirpath = pg_strdup(cwd); } else pg_fatal("You must identify the directory where the %s.\n" @@ -368,16 +378,10 @@ check_required_directory(char **dirpath, char **configpath, } /* - * Trim off any trailing path separators because we construct paths by - * appending to this path. + * Clean up the path, in particular trimming any trailing path separators, + * because we construct paths by appending to this path. */ -#ifndef WIN32 - if ((*dirpath)[strlen(*dirpath) - 1] == '/') -#else - if ((*dirpath)[strlen(*dirpath) - 1] == '/' || - (*dirpath)[strlen(*dirpath) - 1] == '\\') -#endif - (*dirpath)[strlen(*dirpath) - 1] = 0; + canonicalize_path(*dirpath); } /* @@ -386,6 +390,10 @@ check_required_directory(char **dirpath, char **configpath, * If a configuration-only directory was specified, find the real data dir * by querying the running server. This has limited checking because we * can't check for a running server because we can't find postmaster.pid. + * + * On entry, cluster->pgdata has been set from command line or env variable, + * but cluster->pgconfig isn't set. We fill both variables with corrected + * values. */ void adjust_data_dir(ClusterInfo *cluster) @@ -396,6 +404,9 @@ adjust_data_dir(ClusterInfo *cluster) FILE *fp, *output; + /* Initially assume config dir and data dir are the same */ + cluster->pgconfig = pg_strdup(cluster->pgdata); + /* If there is no postgresql.conf, it can't be a config-only dir */ snprintf(filename, sizeof(filename), "%s/postgresql.conf", cluster->pgconfig); if ((fp = fopen(filename, "r")) == NULL) @@ -462,12 +473,7 @@ get_sock_dir(ClusterInfo *cluster, bool live_check) if (GET_MAJOR_VERSION(cluster->major_version) >= 901) { if (!live_check) - { - /* Use the current directory for the socket */ - cluster->sockdir = pg_malloc(MAXPGPATH); - if (!getcwd(cluster->sockdir, MAXPGPATH)) - pg_fatal("could not determine current directory\n"); - } + cluster->sockdir = user_opts.socketdir; else { /* diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 51bd211d46..611a20768b 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -298,7 +298,8 @@ typedef struct bool check; /* true -> ask user for permission to make * changes */ transferMode transfer_mode; /* copy files or link them? */ - int jobs; + int jobs; /* number of processes/threads to use */ + char *socketdir; /* directory to use for Unix sockets */ } UserOpts; typedef struct @@ -374,7 +375,7 @@ bool pid_lock_file_exists(const char *datadir); /* file.c */ void cloneFile(const char *src, const char *dst, - const char *schemaName, const char *relName); + const char *schemaName, const char *relName); void copyFile(const char *src, const char *dst, const char *schemaName, const char *relName); void linkFile(const char *src, const char *dst, -- 2.40.0