Обсуждение: pg_upgrade - add config directory setting
It would perhaps be useful to add optional --old-confdir and --new-confdir parameters to pg_upgrade. If these parameters are absent then pg_upgrade would work as it does now and assume that the config files are in the datadir. The reason for this suggestion is that packages for Ubuntu (and I suppose Debian and possibly others) place the config files in a different directory than the data files. The Ubuntu packaging, for example, puts all the configuration files in /etc/postgresql/VERSION/main/. If I set the data-directories to /var/lib/postgresql/VERSION/main then pg_upgrade complains about missing config files. If I set the data directories to /etc/postgresql/VERSION/main/ then pg_upgrade complains that the "base" subdirectory is missing. Temporarily symlinking postgresql.conf and pg_hba.conf from the config directory to the data directory allowed the upgrade to run successfully but is a bit more kludgey and non-obvious. Cheers, Steve
On Tue, Sep 27, 2011 at 04:13:41PM -0700, Steve Crawford wrote:
> It would perhaps be useful to add optional --old-confdir and
> --new-confdir parameters to pg_upgrade. If these parameters are absent
> then pg_upgrade would work as it does now and assume that the config
> files are in the datadir.
>
> The reason for this suggestion is that packages for Ubuntu (and I
> suppose Debian and possibly others) place the config files in a
> different directory than the data files.
>
> The Ubuntu packaging, for example, puts all the configuration files in
> /etc/postgresql/VERSION/main/.
>
> If I set the data-directories to /var/lib/postgresql/VERSION/main then
> pg_upgrade complains about missing config files.
>
> If I set the data directories to /etc/postgresql/VERSION/main/ then
> pg_upgrade complains that the "base" subdirectory is missing.
>
> Temporarily symlinking postgresql.conf and pg_hba.conf from the config
> directory to the data directory allowed the upgrade to run successfully
> but is a bit more kludgey and non-obvious.
>
> Cheers,
> Steve
I was just about to submit this suggestion. We do the same on Gentoo, as a
default anyway. (Users can pick their own locations for the configuration files
and data directories.) It would simplify the upgrade process by eliminating two
to four steps. (Symlink/copy configuration files in /etc/postgresql-${SLOT}
to /var/lib/postgresql-${SLOT}, same to $version++, pg_upgrade, remove symlinks.)
--
Mr. Aaron W. Swenson
Pseudonym: TitanOfOld
Gentoo Developer
On tis, 2011-09-27 at 16:13 -0700, Steve Crawford wrote: > It would perhaps be useful to add optional --old-confdir and > --new-confdir parameters to pg_upgrade. If these parameters are absent > then pg_upgrade would work as it does now and assume that the config > files are in the datadir. It should work the same way the postmaster itself works: If the given directory is not a data directory, look for the postgresql.conf file and look there for the location of the data directory.
Excerpts from Peter Eisentraut's message of mié sep 28 04:49:43 -0300 2011: > On tis, 2011-09-27 at 16:13 -0700, Steve Crawford wrote: > > It would perhaps be useful to add optional --old-confdir and > > --new-confdir parameters to pg_upgrade. If these parameters are absent > > then pg_upgrade would work as it does now and assume that the config > > files are in the datadir. > > It should work the same way the postmaster itself works: If the given > directory is not a data directory, look for the postgresql.conf file and > look there for the location of the data directory. So we need a postmaster switch: postmaster --parse-config-and-report=data_directory -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On 09/28/2011 12:49 AM, Peter Eisentraut wrote: > On tis, 2011-09-27 at 16:13 -0700, Steve Crawford wrote: >> It would perhaps be useful to add optional --old-confdir and >> --new-confdir parameters to pg_upgrade. If these parameters are absent >> then pg_upgrade would work as it does now and assume that the config >> files are in the datadir. > It should work the same way the postmaster itself works: If the given > directory is not a data directory, look for the postgresql.conf file and > look there for the location of the data directory. > > > > That would make sense to me (I actually tried setting the datadirs based on that assumption). It would require adding that feature to pg_upgrade and tweaking the docs for --XXX-datadir but would not require any new parameters. Cheers, Steve
On ons, 2011-09-28 at 11:53 -0300, Alvaro Herrera wrote: > Excerpts from Peter Eisentraut's message of mié sep 28 04:49:43 -0300 2011: > > On tis, 2011-09-27 at 16:13 -0700, Steve Crawford wrote: > > > It would perhaps be useful to add optional --old-confdir and > > > --new-confdir parameters to pg_upgrade. If these parameters are absent > > > then pg_upgrade would work as it does now and assume that the config > > > files are in the datadir. > > > > It should work the same way the postmaster itself works: If the given > > directory is not a data directory, look for the postgresql.conf file and > > look there for the location of the data directory. > > So we need a postmaster switch: > > postmaster --parse-config-and-report=data_directory Perhaps. That might have some use for pg_ctl as well.
Peter Eisentraut wrote: > On ons, 2011-09-28 at 11:53 -0300, Alvaro Herrera wrote: > > Excerpts from Peter Eisentraut's message of mi? sep 28 04:49:43 -0300 2011: > > > On tis, 2011-09-27 at 16:13 -0700, Steve Crawford wrote: > > > > It would perhaps be useful to add optional --old-confdir and > > > > --new-confdir parameters to pg_upgrade. If these parameters are absent > > > > then pg_upgrade would work as it does now and assume that the config > > > > files are in the datadir. > > > > > > It should work the same way the postmaster itself works: If the given > > > directory is not a data directory, look for the postgresql.conf file and > > > look there for the location of the data directory. > > > > So we need a postmaster switch: > > > > postmaster --parse-config-and-report=data_directory > > Perhaps. That might have some use for pg_ctl as well. FYI, unless this is backpatched, which I doubt, it is only going to be available for the _new_ cluster. You are right that while pg_upgrade doesn't care about the location of postgresql.conf and pg_hba.conf, we have to point to those to start the server, and pg_upgrade does need to access some data files, so it also needs to know about the data location. I am unclear how to proceed with this, particularly with the backpatch requirement. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > Peter Eisentraut wrote: > > On ons, 2011-09-28 at 11:53 -0300, Alvaro Herrera wrote: > > > Excerpts from Peter Eisentraut's message of mi? sep 28 04:49:43 -0300 2011: > > > > On tis, 2011-09-27 at 16:13 -0700, Steve Crawford wrote: > > > > > It would perhaps be useful to add optional --old-confdir and > > > > > --new-confdir parameters to pg_upgrade. If these parameters are absent > > > > > then pg_upgrade would work as it does now and assume that the config > > > > > files are in the datadir. > > > > > > > > It should work the same way the postmaster itself works: If the given > > > > directory is not a data directory, look for the postgresql.conf file and > > > > look there for the location of the data directory. > > > > > > So we need a postmaster switch: > > > > > > postmaster --parse-config-and-report=data_directory > > > > Perhaps. That might have some use for pg_ctl as well. > > FYI, unless this is backpatched, which I doubt, it is only going to be > available for the _new_ cluster. > > You are right that while pg_upgrade doesn't care about the location of > postgresql.conf and pg_hba.conf, we have to point to those to start the > server, and pg_upgrade does need to access some data files, so it also > needs to know about the data location. > > I am unclear how to proceed with this, particularly with the backpatch > requirement. Thinking some more, I don't need to know the data directory while the server is down --- I already am starting it. pg_upgrade starts both old and new servers during its check phase, and it could look up the data_directory GUC variable and use that value when accessing files. That would work for old and new servers. However, I assume that is something we would not backpatch to 9.1. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Excerpts from Bruce Momjian's message of jue sep 29 09:56:09 -0300 2011: > > Thinking some more, I don't need to know the data directory while the > server is down --- I already am starting it. pg_upgrade starts both old > and new servers during its check phase, and it could look up the > data_directory GUC variable and use that value when accessing files. > That would work for old and new servers. However, I assume that is > something we would not backpatch to 9.1. Why not? We've supported separate data/config dirs for a long time now, so it seems to me that pg_upgrade not coping with them is a bug. If pg_upgrade starts postmaster, it seems simple to grab the data_directory setting, is it not? -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Thu, Sep 29, 2011 at 10:44:29AM -0300, Alvaro Herrera wrote: > > Excerpts from Bruce Momjian's message of jue sep 29 09:56:09 -0300 2011: > > > > Thinking some more, I don't need to know the data directory while the > > server is down --- I already am starting it. pg_upgrade starts both old > > and new servers during its check phase, and it could look up the > > data_directory GUC variable and use that value when accessing files. > > That would work for old and new servers. However, I assume that is > > something we would not backpatch to 9.1. > > Why not? We've supported separate data/config dirs for a long time now, > so it seems to me that pg_upgrade not coping with them is a bug. If > pg_upgrade starts postmaster, it seems simple to grab the data_directory > setting, is it not? I was going to say. I'd view this as bringing the behavior of pg_upgrade to a consistent state with postgres. I vote for it being backpatched to 9.0 even. For whatever my vote is worth. -- Mr. Aaron W. Swenson Pseudonym: TitanOfOld Gentoo Developer
Mr. Aaron W. Swenson wrote: -- Start of PGP signed section. > On Thu, Sep 29, 2011 at 10:44:29AM -0300, Alvaro Herrera wrote: > > > > Excerpts from Bruce Momjian's message of jue sep 29 09:56:09 -0300 2011: > > > > > > Thinking some more, I don't need to know the data directory while the > > > server is down --- I already am starting it. pg_upgrade starts both old > > > and new servers during its check phase, and it could look up the > > > data_directory GUC variable and use that value when accessing files. > > > That would work for old and new servers. However, I assume that is > > > something we would not backpatch to 9.1. > > > > Why not? We've supported separate data/config dirs for a long time now, > > so it seems to me that pg_upgrade not coping with them is a bug. If > > pg_upgrade starts postmaster, it seems simple to grab the data_directory > > setting, is it not? > > I was going to say. I'd view this as bringing the behavior of pg_upgrade > to a consistent state with postgres. I vote for it being backpatched to > 9.0 even. For whatever my vote is worth. Well, I would call it more of a limitation of pg_upgrade, rather than a bug --- perhaps documenting the limitation in the back branches is sufficient. I think the only big argument for backpatching the feature is that 9.1 is the first release that packagers are going to use pg_upgrade, and fixing it now makes sense because it avoids packagers from implementing a workaround on every platform that will go away with 9.2. So, there are four options: 1 document the limitation and require users to use symlinks 2 add a --old/new-configdir parameter to pg_upgrade 3 have pg_upgrade find the real data dir by starting the server 4 add a flag to some tool to return the real data dir, and backpatch that #4 has the problem of backpatching. I looked at #3 and the first thing pg_upgrade does is to read PG_VERSION, and the second thing is to call pg_controldata, both of which need the real data dir, so it is going to require some major code ordering adjustments to do #3. One interesting trick would be to start the old and new servers to pull the data dir at the start only if we don't see PG_VERSION in the specified data directory --- that would limit the overhead of starting the servers, and limit the risk for backpatching. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 09/29/2011 08:20 AM, Bruce Momjian wrote: > ... > 1 document the limitation and require users to use symlinks > 2 add a --old/new-configdir parameter to pg_upgrade > 3 have pg_upgrade find the real data dir by starting the server > 4 add a flag to some tool to return the real data dir, and backpatch > that 5. (really 3a). Have pg_upgrade itself check the specified --XXX-datadir for postgresql.conf and use the data_directory setting therein using the same rules as followed by the server. This would mean that there are no new options to pg_upgrade and that pg_upgrade operation would not change when postgresql.conf is in the data-directory. This would also make it consistent with PostgreSQL's notion of file-locations: "If you wish to keep the configuration files elsewhere than the data directory, the postgres -D command-line option or PGDATA environment variable must point to the directory containing the configuration files, and the data_directory parameter must be set in postgresql.conf..." So for backporting, it could just be considered a "bug fix" that aligns pg_upgrade's interpretation of datadir to that of the server. Cheers, Steve
Steve Crawford wrote: > On 09/29/2011 08:20 AM, Bruce Momjian wrote: > > ... > > 1 document the limitation and require users to use symlinks > > 2 add a --old/new-configdir parameter to pg_upgrade > > 3 have pg_upgrade find the real data dir by starting the server > > 4 add a flag to some tool to return the real data dir, and backpatch > > that > 5. (really 3a). Have pg_upgrade itself check the specified --XXX-datadir > for postgresql.conf and use the data_directory setting therein using the > same rules as followed by the server. > > This would mean that there are no new options to pg_upgrade and that > pg_upgrade operation would not change when postgresql.conf is in the > data-directory. This would also make it consistent with PostgreSQL's > notion of file-locations: > > "If you wish to keep the configuration files elsewhere than the data > directory, the postgres -D command-line option or PGDATA environment > variable must point to the directory containing the configuration files, > and the data_directory parameter must be set in postgresql.conf..." > > So for backporting, it could just be considered a "bug fix" that aligns > pg_upgrade's interpretation of datadir to that of the server. pg_upgrade is not about to start reading through postgresql.conf looking for a definition for data_directory --- there are too many cases where this could go wrong. It would need a full postgresql.conf parser. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> pg_upgrade is not about to start reading through postgresql.conf looking
> for a definition for data_directory --- there are too many cases where
> this could go wrong. It would need a full postgresql.conf parser.
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. We've had requests for that
type of functionality before, IIRC. The --describe-config option does
something related, but not what's needed here.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > pg_upgrade is not about to start reading through postgresql.conf looking > > for a definition for data_directory --- there are too many cases where > > this could go wrong. It would need a full postgresql.conf parser. > > 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. We've had requests for that > type of functionality before, IIRC. The --describe-config option does > something related, but not what's needed here. 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. We could minimize that by using this feature only if postgresql.conf exists in the specified data directory but PG_VERSION does not. Adding this features is similar to this TODO item: Allow configuration files to be independently validated This still seems like a lot to backpatch. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
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.
regards, tom lane
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. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
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)
Bruce Momjian wrote: > 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. OK, I have modified the postmaster in PG 9.2 to allow output of the data directory, and modified pg_ctl to use that, so starting in PG 9.2 pg_ctl will work cleanly for config-only directories. I will now work on pg_upgrade to also use the new flag to find the data directory from a config-only install. However, this is only available in PG 9.2, and it will only be in PG 9.3 that you can hope to use this feature (if old is PG 9.2 or later). I am afraid the symlink hack will have to be used for several more years, and if you are supporting upgrades from pre-9.2, perhaps forever. I did find that it is possible to use pg_ctl -w start on a config-only install using this trick: su -l postgres \ -c "env PGPORT=\"5432\" /usr/lib/postgresql-9.1/bin/pg_ctl start -w \ -t 60 -s -D /var/lib/postgresql/9.1/data/\ -o '-D /etc/postgresql-9.1/ \ --data-directory=/var/lib/postgresql/9.1/data/\ --silent-mode=true'" Unfortunately pg_upgrade doesn't support the -o option which would make this possible for pg_upgrade. One idea would be to add -o/-O options to pg_upgrade 9.2 to allow this to work even with old installs, but frankly, this is so confusing I am not sure we want to encourage people to do things like this. Of course, the symlink hack is even worse, so maybe there is some merit to this. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > I will now work on pg_upgrade to also use the new flag to find the data > directory from a config-only install. However, this is only available > in PG 9.2, and it will only be in PG 9.3 that you can hope to use this > feature (if old is PG 9.2 or later). I am afraid the symlink hack will > have to be used for several more years, and if you are supporting > upgrades from pre-9.2, perhaps forever. The attached patch uses "postmaster -C data_directory" to allow config-only upgrades. It will allow a normal 9.1 cluster to be upgraded to a 9.2 config-only cluster. -- 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..3ab1b5c *** 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,381 ---- #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]; + char cmd[MAXPGPATH], cmd_output[MAX_STRING]; + FILE *fd, *output; + + /* 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 find the real data directory. */ + + prep_status("Finding the real data directory for the %s cluster", + CLUSTER_NAME(cluster)); + + /* + * We don't have a data directory yet, so we can't check the PG + * version, so this might fail --- only works for PG 9.2+. If this + * fails, pg_upgrade will fail anyway because the data files will not + * be found. + */ + snprintf(cmd, sizeof(cmd), "\"%s/postmaster\" -D \"%s\" -C data_directory", + cluster->bindir, cluster->pgconfig); + + if ((output = popen(cmd, "r")) == NULL || + fgets(cmd_output, sizeof(cmd_output), output) == NULL) + pg_log(PG_FATAL, "Could not get data directory using %s: %s\n", + cmd, getErrorText(errno)); + + pclose(output); + + /* Remove trailing newline */ + if (strchr(cmd_output, '\n') != NULL) + *strchr(cmd_output, '\n') = '\0'; + + cluster->pgdata = pg_strdup(cmd_output); + + 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..d512ef3 *** a/contrib/pg_upgrade/server.c --- b/contrib/pg_upgrade/server.c *************** start_postmaster(ClusterInfo *cluster) *** 169,175 **** 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", --- 169,175 ---- 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) *** 208,224 **** { char cmd[MAXPGPATH]; const char *bindir; ! const char *datadir; if (os_info.running_cluster == &old_cluster) { bindir = old_cluster.bindir; ! datadir = old_cluster.pgdata; } else if (os_info.running_cluster == &new_cluster) { bindir = new_cluster.bindir; ! datadir = new_cluster.pgdata; } else return; /* no cluster running */ --- 208,224 ---- { char cmd[MAXPGPATH]; const char *bindir; ! const char *configdir; if (os_info.running_cluster == &old_cluster) { bindir = old_cluster.bindir; ! configdir = old_cluster.pgconfig; } else if (os_info.running_cluster == &new_cluster) { bindir = new_cluster.bindir; ! configdir = new_cluster.pgconfig; } else return; /* no cluster running */ *************** stop_postmaster(bool fast) *** 226,232 **** snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" %s stop >> " "\"%s\" 2>&1" SYSTEMQUOTE, ! bindir, log_opts.filename2, datadir, fast ? "-m fast" : "", log_opts.filename2); exec_prog(fast ? false : true, "%s", cmd); --- 226,232 ---- snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" %s stop >> " "\"%s\" 2>&1" SYSTEMQUOTE, ! bindir, log_opts.filename2, configdir, fast ? "-m fast" : "", log_opts.filename2); exec_prog(fast ? false : true, "%s", cmd);
Bruce Momjian wrote: > Bruce Momjian wrote: > > I will now work on pg_upgrade to also use the new flag to find the data > > directory from a config-only install. However, this is only available > > in PG 9.2, and it will only be in PG 9.3 that you can hope to use this > > feature (if old is PG 9.2 or later). I am afraid the symlink hack will > > have to be used for several more years, and if you are supporting > > upgrades from pre-9.2, perhaps forever. > > The attached patch uses "postmaster -C data_directory" to allow > config-only upgrades. It will allow a normal 9.1 cluster to be upgraded > to a 9.2 config-only cluster. Applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > OK, I have modified the postmaster in PG 9.2 to allow output of the data > directory, and modified pg_ctl to use that, so starting in PG 9.2 pg_ctl > will work cleanly for config-only directories. > > I will now work on pg_upgrade to also use the new flag to find the data > directory from a config-only install. However, this is only available > in PG 9.2, and it will only be in PG 9.3 that you can hope to use this > feature (if old is PG 9.2 or later). I am afraid the symlink hack will > have to be used for several more years, and if you are supporting > upgrades from pre-9.2, perhaps forever. > > I did find that it is possible to use pg_ctl -w start on a config-only > install using this trick: > > su -l postgres \ > -c "env PGPORT=\"5432\" /usr/lib/postgresql-9.1/bin/pg_ctl start -w \ > -t 60 -s -D /var/lib/postgresql/9.1/data/ \ > -o '-D /etc/postgresql-9.1/ \ > --data-directory=/var/lib/postgresql/9.1/data/ \ > --silent-mode=true'" > > Unfortunately pg_upgrade doesn't support the -o option which would make > this possible for pg_upgrade. > > One idea would be to add -o/-O options to pg_upgrade 9.2 to allow this > to work even with old installs, but frankly, this is so confusing I am > not sure we want to encourage people to do things like this. Of course, > the symlink hack is even worse, so maybe there is some merit to this. OK, the attached patch adds -o/-O options to pg_upgrade to mimick pg_ctl -o, and documents the 'Gentoo method' for allowing pg_upgrade to handle pre-9.2 upgrades for config-only installs. I think this closes the issue, with no backpatching required for it to work for new PG 9.2. Users will have to continue using the symlink method for new PG 9.1. -- 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 3ab1b5c..9892b97 *** a/contrib/pg_upgrade/option.c --- b/contrib/pg_upgrade/option.c *************** parseCommandLine(int argc, char *argv[]) *** 39,44 **** --- 39,46 ---- {"new-datadir", required_argument, NULL, 'D'}, {"old-bindir", required_argument, NULL, 'b'}, {"new-bindir", required_argument, NULL, 'B'}, + {"old-options", required_argument, NULL, 'o'}, + {"new-options", required_argument, NULL, 'O'}, {"old-port", required_argument, NULL, 'p'}, {"new-port", required_argument, NULL, 'P'}, *************** parseCommandLine(int argc, char *argv[]) *** 93,99 **** getcwd(os_info.cwd, MAXPGPATH); ! while ((option = getopt_long(argc, argv, "d:D:b:B:cgG:kl:p:P:u:v", long_options, &optindex)) != -1) { switch (option) --- 95,101 ---- getcwd(os_info.cwd, MAXPGPATH); ! while ((option = getopt_long(argc, argv, "d:D:b:B:cgG:kl:o:O:p:P:u:v", long_options, &optindex)) != -1) { switch (option) *************** parseCommandLine(int argc, char *argv[]) *** 141,146 **** --- 143,161 ---- log_opts.filename = pg_strdup(optarg); break; + case 'o': + old_cluster.pgopts = pg_strdup(optarg); + break; + + case 'O': + new_cluster.pgopts = pg_strdup(optarg); + break; + + /* + * Someday, the port number option could be removed and + * passed using -o/-O, but that requires postmaster -C + * to be supported on all old/new versions. + */ case 'p': if ((old_cluster.port = atoi(optarg)) <= 0) { *************** Options:\n\ *** 242,247 **** --- 257,264 ---- -G, --debugfile=FILENAME output debugging activity to file\n\ -k, --link link instead of copying files to new cluster\n\ -l, --logfile=FILENAME log session activity to file\n\ + -o, --old-options=OPTIONS old cluster options to pass to the server\n\ + -O, --new-options=OPTIONS new cluster options to pass to the server\n\ -p, --old-port=OLDPORT old cluster port number (default %d)\n\ -P, --new-port=NEWPORT new cluster port number (default %d)\n\ -u, --user=NAME clusters superuser (default \"%s\")\n\ diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h new file mode 100644 index 0fb16ed..b7e4ea5 *** a/contrib/pg_upgrade/pg_upgrade.h --- b/contrib/pg_upgrade/pg_upgrade.h *************** typedef struct *** 189,194 **** --- 189,195 ---- 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 */ + char *pgopts; /* options to pass to the server, like pg_ctl -o */ unsigned short port; /* port number where postmaster is waiting */ uint32 major_version; /* PG_VERSION of cluster */ char major_version_str[64]; /* string PG_VERSION of cluster */ diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c new file mode 100644 index d512ef3..9c4f2d6 *** a/contrib/pg_upgrade/server.c --- b/contrib/pg_upgrade/server.c *************** start_postmaster(ClusterInfo *cluster) *** 168,179 **** */ 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", ! log_opts.filename2); /* * Don't throw an error right away, let connecting throw the error because --- 168,179 ---- */ snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" " ! "-o \"-p %d %s %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", ! cluster->pgopts ? cluster->pgopts : "", log_opts.filename2); /* * Don't throw an error right away, let connecting throw the error because *************** void *** 207,233 **** stop_postmaster(bool fast) { char cmd[MAXPGPATH]; ! const char *bindir; ! const char *configdir; if (os_info.running_cluster == &old_cluster) ! { ! bindir = old_cluster.bindir; ! configdir = old_cluster.pgconfig; ! } else if (os_info.running_cluster == &new_cluster) ! { ! bindir = new_cluster.bindir; ! configdir = new_cluster.pgconfig; ! } else ! return; /* no cluster running */ snprintf(cmd, sizeof(cmd), ! SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" %s stop >> " ! "\"%s\" 2>&1" SYSTEMQUOTE, ! bindir, log_opts.filename2, configdir, fast ? "-m fast" : "", ! log_opts.filename2); exec_prog(fast ? false : true, "%s", cmd); --- 207,227 ---- stop_postmaster(bool fast) { char cmd[MAXPGPATH]; ! ClusterInfo *cluster; if (os_info.running_cluster == &old_cluster) ! cluster = &old_cluster; else if (os_info.running_cluster == &new_cluster) ! cluster = &new_cluster; else ! return; /* no cluster running */ snprintf(cmd, sizeof(cmd), ! SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"%s\" " ! "%s stop >> \"%s\" 2>&1" SYSTEMQUOTE, ! cluster->bindir, log_opts.filename2, cluster->pgconfig, ! cluster->pgopts ? cluster->pgopts : "", ! fast ? "-m fast" : "", log_opts.filename2); exec_prog(fast ? false : true, "%s", cmd); diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index 0d08424..460d06b *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *************** *** 115,120 **** --- 115,134 ---- </varlistentry> <varlistentry> + <term><option>-o</option> <replaceable class="parameter">options</replaceable></term> + <term><option>--old-options</option> <replaceable class="parameter">options</replaceable></term> + <listitem><para>options to be passed directly to the + old <command>postgres</command> command</para></listitem> + </varlistentry> + + <varlistentry> + <term><option>-O</option> <replaceable class="parameter">options</replaceable></term> + <term><option>--new-options</option> <replaceable class="parameter">options</replaceable></term> + <listitem><para>options to be passed directly to the + new <command>postgres</command> command</para></listitem> + </varlistentry> + + <varlistentry> <term><option>-p</option> <replaceable>old_port_number</></term> <term><option>--old-port=</option><replaceable>old_portnum</></term> <listitem><para>the old cluster port number; environment *************** psql --username postgres --file script.s *** 560,565 **** --- 574,587 ---- </para> <para> + If you are upgrading a pre-<productname>PostgreSQL</> 9.2 cluster + that uses a configuration-file-only directory, you must pass the + real data directory location to <application>pg_upgrade</>, and + pass the configuration directory location to the server, e.g. + <literal>-d /real-data-directory -o '-D /configuration-directory'</>. + </para> + + <para> If you want to use link mode and you don't want your old cluster to be modified when the new cluster is started, make a copy of the old cluster and upgrade that with link mode. To make a valid copy
Bruce Momjian wrote: > Bruce Momjian wrote: > > OK, I have modified the postmaster in PG 9.2 to allow output of the data > > directory, and modified pg_ctl to use that, so starting in PG 9.2 pg_ctl > > will work cleanly for config-only directories. > > > > I will now work on pg_upgrade to also use the new flag to find the data > > directory from a config-only install. However, this is only available > > in PG 9.2, and it will only be in PG 9.3 that you can hope to use this > > feature (if old is PG 9.2 or later). I am afraid the symlink hack will > > have to be used for several more years, and if you are supporting > > upgrades from pre-9.2, perhaps forever. > > > > I did find that it is possible to use pg_ctl -w start on a config-only > > install using this trick: > > > > su -l postgres \ > > -c "env PGPORT=\"5432\" /usr/lib/postgresql-9.1/bin/pg_ctl start -w \ > > -t 60 -s -D /var/lib/postgresql/9.1/data/ \ > > -o '-D /etc/postgresql-9.1/ \ > > --data-directory=/var/lib/postgresql/9.1/data/ \ > > --silent-mode=true'" > > > > Unfortunately pg_upgrade doesn't support the -o option which would make > > this possible for pg_upgrade. > > > > One idea would be to add -o/-O options to pg_upgrade 9.2 to allow this > > to work even with old installs, but frankly, this is so confusing I am > > not sure we want to encourage people to do things like this. Of course, > > the symlink hack is even worse, so maybe there is some merit to this. > > OK, the attached patch adds -o/-O options to pg_upgrade to mimick pg_ctl > -o, and documents the 'Gentoo method' for allowing pg_upgrade to handle > pre-9.2 upgrades for config-only installs. I think this closes the > issue, with no backpatching required for it to work for new PG 9.2. > Users will have to continue using the symlink method for new PG 9.1. Applied to head. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +