Обсуждение: pg_upgrade using appname to lock out other users
You might remember we added a postmaster/postgres -b switch to indicate binary upgrade mode. The attached patch prevents any client without an application_name of 'binary-upgrade' from connecting to the cluster while it is binary upgrade mode. This helps prevent unauthorized users from connecting during the upgrade. This will not help for clusters that do not have the -b flag, e.g. pre-9.1. Does this seem useful? Something for 9.1 or 9.2? This idea came from Andrew Dunstan via IRC during a pg_upgrade run by Stephen Frost when some clients accidentally connected. (Stephen reran pg_upgrade successfully.) -- 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/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c new file mode 100644 index e329dc3..0b6fb61 *** a/contrib/pg_upgrade/pg_upgrade.c --- b/contrib/pg_upgrade/pg_upgrade.c *************** setup(char *argv0, bool live_check) *** 171,176 **** --- 171,178 ---- *last_dir_separator(exec_path) = '\0'; canonicalize_path(exec_path); os_info.exec_path = pg_strdup(exec_path); + + pg_putenv("PGAPPNAME", "binary-upgrade"); } diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c new file mode 100644 index 8347f52..f359af2 *** a/src/backend/utils/init/postinit.c --- b/src/backend/utils/init/postinit.c *************** InitPostgres(const char *in_dbname, Oid *** 833,838 **** --- 833,848 ---- if (MyProcPort != NULL) process_startup_options(MyProcPort, am_superuser); + /* + * Binary upgrades only allow the proper application name + */ + if (IsBinaryUpgrade && strcmp(application_name, "binary-upgrade") != 0) + { + ereport(FATAL, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("the application name must be \"binary-upgrade\" to connect in binary upgrade mode"))); + } + /* Process pg_db_role_setting options */ process_settings(MyDatabaseId, GetSessionUserId());
Bruce Momjian <bruce@momjian.us> writes: > You might remember we added a postmaster/postgres -b switch to indicate > binary upgrade mode. The attached patch prevents any client without an > application_name of 'binary-upgrade' from connecting to the cluster > while it is binary upgrade mode. This helps prevent unauthorized users > from connecting during the upgrade. This will not help for clusters > that do not have the -b flag, e.g. pre-9.1. > Does this seem useful? No ... that seems like a kluge. It's ugly and it's leaky. What we really ought to be doing here is fixing things so that pg_upgrade does not need to have a running postmaster in either installation, but works with some variant of standalone mode. That would actually be *safe* against concurrent connections, rather than only sorta kinda maybe safe. regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > You might remember we added a postmaster/postgres -b switch to indicate > > binary upgrade mode. The attached patch prevents any client without an > > application_name of 'binary-upgrade' from connecting to the cluster > > while it is binary upgrade mode. This helps prevent unauthorized users > > from connecting during the upgrade. This will not help for clusters > > that do not have the -b flag, e.g. pre-9.1. > > > Does this seem useful? > > No ... that seems like a kluge. It's ugly and it's leaky. > > What we really ought to be doing here is fixing things so that > pg_upgrade does not need to have a running postmaster in either > installation, but works with some variant of standalone mode. > That would actually be *safe* against concurrent connections, > rather than only sorta kinda maybe safe. I keep replying to that suggestion by reminding people that pg_upgrade relies heavily on psql features, as does pg_dumpall, and recoding that in the backend will be error-prone. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 2011-06-15 05:01, Bruce Momjian wrote: > You might remember we added a postmaster/postgres -b switch to indicate > binary upgrade mode. The attached patch prevents any client without an > application_name of 'binary-upgrade' from connecting to the cluster > while it is binary upgrade mode. This helps prevent unauthorized users > from connecting during the upgrade. This will not help for clusters > that do not have the -b flag, e.g. pre-9.1. > > Does this seem useful? Something for 9.1 or 9.2? > > This idea came from Andrew Dunstan via IRC during a pg_upgrade run by > Stephen Frost when some clients accidentally connected. (Stephen reran > pg_upgrade successfully.) Couldn't the -b flag also imply a very strict hba.conf configuration, that essentially only lets pg_upgrade in..? -- Jesper
Jesper Krogh wrote: > On 2011-06-15 05:01, Bruce Momjian wrote: > > You might remember we added a postmaster/postgres -b switch to indicate > > binary upgrade mode. The attached patch prevents any client without an > > application_name of 'binary-upgrade' from connecting to the cluster > > while it is binary upgrade mode. This helps prevent unauthorized users > > from connecting during the upgrade. This will not help for clusters > > that do not have the -b flag, e.g. pre-9.1. > > > > Does this seem useful? Something for 9.1 or 9.2? > > > > This idea came from Andrew Dunstan via IRC during a pg_upgrade run by > > Stephen Frost when some clients accidentally connected. (Stephen reran > > pg_upgrade successfully.) > Couldn't the -b flag also imply a very strict hba.conf configuration, that > essentially only lets pg_upgrade in..? Yes, it could. What rules would we use? We could prohibit non-local connections. -- 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: > > > You might remember we added a postmaster/postgres -b switch to indicate > > > binary upgrade mode. The attached patch prevents any client without an > > > application_name of 'binary-upgrade' from connecting to the cluster > > > while it is binary upgrade mode. This helps prevent unauthorized users > > > from connecting during the upgrade. This will not help for clusters > > > that do not have the -b flag, e.g. pre-9.1. > > > > > Does this seem useful? > > > > No ... that seems like a kluge. It's ugly and it's leaky. > > > > What we really ought to be doing here is fixing things so that > > pg_upgrade does not need to have a running postmaster in either > > installation, but works with some variant of standalone mode. > > That would actually be *safe* against concurrent connections, > > rather than only sorta kinda maybe safe. > > I keep replying to that suggestion by reminding people that pg_upgrade > relies heavily on psql features, as does pg_dumpall, and recoding that > in the backend will be error-prone. Also, a standalone backend does not have libpq either so how do you get values into application variables? Parse the text output? That seems like a much larger kludge. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Wed, Jun 15, 2011 at 8:05 AM, Bruce Momjian <bruce@momjian.us> wrote: > Bruce Momjian wrote: >> Tom Lane wrote: >> > Bruce Momjian <bruce@momjian.us> writes: >> > > You might remember we added a postmaster/postgres -b switch to indicate >> > > binary upgrade mode. The attached patch prevents any client without an >> > > application_name of 'binary-upgrade' from connecting to the cluster >> > > while it is binary upgrade mode. This helps prevent unauthorized users >> > > from connecting during the upgrade. This will not help for clusters >> > > that do not have the -b flag, e.g. pre-9.1. >> > >> > > Does this seem useful? >> > >> > No ... that seems like a kluge. It's ugly and it's leaky. >> > >> > What we really ought to be doing here is fixing things so that >> > pg_upgrade does not need to have a running postmaster in either >> > installation, but works with some variant of standalone mode. >> > That would actually be *safe* against concurrent connections, >> > rather than only sorta kinda maybe safe. >> >> I keep replying to that suggestion by reminding people that pg_upgrade >> relies heavily on psql features, as does pg_dumpall, and recoding that >> in the backend will be error-prone. > > Also, a standalone backend does not have libpq either so how do you get > values into application variables? Parse the text output? That seems > like a much larger kludge. Maybe we could do something like this. 1. pg_upgrade invokes the postmaster with --binary-upgrade=<port>:<password> 2. postmaster starts up into multi-user mode, but it does not start autovacuum and ignores pg_hba.conf, listen_addresses, and port. Instead it listens only on the localhost interface on the designated port (perhaps the port can be a filename on systems that support UNIX sockets, and it can listen only on a UNIX socket at that location instead). It refuses all connections except for those that attempt to log in with binary_upgrade as the user and the given password as the password. pg_upgrade will randomly generate a password (like C51622FA-7513-4300-A4B7-6423769276F8) and port number at the start of each run, and use that for all connections to the postmaster. As a separate issue, I tend to agree with Tom that using psql as part of the pg_upgrade process is a lousy idea and we need a better solution. But let's fix one thing at a time. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 06/14/2011 11:01 PM, Bruce Momjian wrote: > You might remember we added a postmaster/postgres -b switch to indicate > binary upgrade mode. The attached patch prevents any client without an > application_name of 'binary-upgrade' from connecting to the cluster > while it is binary upgrade mode. This helps prevent unauthorized users > from connecting during the upgrade. This will not help for clusters > that do not have the -b flag, e.g. pre-9.1. > > Does this seem useful? Something for 9.1 or 9.2? > > This idea came from Andrew Dunstan via IRC during a pg_upgrade run by > Stephen Frost when some clients accidentally connected. (Stephen reran > pg_upgrade successfully.) > What I actually had in mind was rather different: an HBA mechanism based on appname. But on second thoughts maybe the protocol wouldn't support that. cheers andrew
On Wed, Jun 15, 2011 at 10:05 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > What I actually had in mind was rather different: an HBA mechanism based on > appname. But on second thoughts maybe the protocol wouldn't support that. Ah, a similar thought struck me. Independent of this particular feature, it would be rather useful to augment pg_hba.conf to filter based on appname. For my "pet" case, that would mean one might let "slon" and "slonik" (Slony appname values) in, whilst keeping other appnames out, during a system maintenance. It's debatable whether or not that's more useful than filtering on the basis of user names, which are likely to be pretty nearly isomorphic to appnames. Due to the near-isomorphism, I would not be comfortable trying to claim that this is anywhere near essential. During upgrade, I expect that we'd want everything but the upgrade process locked out, including some backend-ish processes such as autovacuum. That doesn't have quite the same "feel" as pg_hba.conf, which also piques my "not totally comfortable" with this being a pg_hba.conf thing. That doesn't mean the idea's useless in and of itself, nor that there's not some useful adaption to be made. -- When confronted by a difficult problem, solve it by reducing it to the question, "How would the Lone Ranger handle this?"
On Wed, Jun 15, 2011 at 10:05 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > What I actually had in mind was rather different: an HBA mechanism based on > appname. But on second thoughts maybe the protocol wouldn't support that. Ah, a similar thought struck me. Independent of this particular feature, it would be rather useful to augment pg_hba.conf to filter based on appname. For my "pet" case, that would mean one might let "slon" and "slonik" (Slony appname values) in, whilst keeping other appnames out, during a system maintenance. It's debatable whether or not that's more useful than filtering on the basis of user names, which are likely to be pretty nearly isomorphic to appnames. Due to the near-isomorphism, I would not be comfortable trying to claim that this is anywhere near essential. During upgrade, I expect that we'd want everything but the upgrade process locked out, including some backend-ish processes such as autovacuum. That doesn't have quite the same "feel" as pg_hba.conf, which also piques my "not totally comfortable" with this being a pg_hba.conf thing. That doesn't mean the idea's useless in and of itself, nor that there's not some useful adaption to be made. -- When confronted by a difficult problem, solve it by reducing it to the question, "How would the Lone Ranger handle this?"
Excerpts from Robert Haas's message of mié jun 15 08:45:21 -0400 2011: > 1. pg_upgrade invokes the postmaster with --binary-upgrade=<port>:<password> > > 2. postmaster starts up into multi-user mode, but it does not start > autovacuum and ignores pg_hba.conf, listen_addresses, and port. > Instead it listens only on the localhost interface on the designated > port (perhaps the port can be a filename on systems that support UNIX > sockets, and it can listen only on a UNIX socket at that location > instead). It refuses all connections except for those that attempt to > log in with binary_upgrade as the user and the given password as the > password. pg_upgrade will randomly generate a password (like > C51622FA-7513-4300-A4B7-6423769276F8) and port number at the start of > each run, and use that for all connections to the postmaster. Seems good, except that passing the password as a command line argument is obviously broken from a privacy perspective -- anyone could see the process list and get it. Maybe have postmaster ask for it on startup somehow, or have pg_upgrade write it in a file which is read by postmaster. > As a separate issue, I tend to agree with Tom that using psql as part > of the pg_upgrade process is a lousy idea and we need a better > solution. But let's fix one thing at a time. Agreed on both counts ... but ... does this mean that we need a different program for programmable tasks as opposed to interactive ones? Dealing with standalone backends *is* a pain, that's for sure. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Wed, Jun 15, 2011 at 12:12 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Robert Haas's message of mié jun 15 08:45:21 -0400 2011: > Seems good, except that passing the password as a command line argument > is obviously broken from a privacy perspective -- anyone could see the > process list and get it. Maybe have postmaster ask for it on startup > somehow, or have pg_upgrade write it in a file which is read by > postmaster. Writing it to a file which is ready by postmaster seems promising. Then you wouldn't even need a command line option; you could just have the postmaster write out binary_upgrade.conf and have that work like recovery.conf to trigger the system to start up in a different mode. >> As a separate issue, I tend to agree with Tom that using psql as part >> of the pg_upgrade process is a lousy idea and we need a better >> solution. But let's fix one thing at a time. > > Agreed on both counts ... but ... does this mean that we need a > different program for programmable tasks as opposed to interactive > ones? Dealing with standalone backends *is* a pain, that's for sure. I'm not sure exactly what is needed here - what programmable tasks are you thinking of, other than pg_upgrade? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Alvaro Herrera <alvherre@commandprompt.com> writes: > Excerpts from Robert Haas's message of mié jun 15 08:45:21 -0400 2011: >> As a separate issue, I tend to agree with Tom that using psql as part >> of the pg_upgrade process is a lousy idea and we need a better >> solution. But let's fix one thing at a time. > Agreed on both counts ... but ... does this mean that we need a > different program for programmable tasks as opposed to interactive > ones? Dealing with standalone backends *is* a pain, that's for sure. So we fix the interface presented by standalone mode to be less insane. That way, we can *reduce* the net amount of cruft in the system, rather than adding more as all these proposals do. regards, tom lane
Excerpts from Robert Haas's message of mié jun 15 12:51:29 -0400 2011: > On Wed, Jun 15, 2011 at 12:12 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > Agreed on both counts ... but ... does this mean that we need a > > different program for programmable tasks as opposed to interactive > > ones? Dealing with standalone backends *is* a pain, that's for sure. > > I'm not sure exactly what is needed here - what programmable tasks are > you thinking of, other than pg_upgrade? Well, something to use on shell scripts and the like first and foremost; see http://petereisentraut.blogspot.com/2010/03/running-sql-scripts-with-psql.html -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Excerpts from Tom Lane's message of mié jun 15 12:52:30 -0400 2011: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Excerpts from Robert Haas's message of mié jun 15 08:45:21 -0400 2011: > >> As a separate issue, I tend to agree with Tom that using psql as part > >> of the pg_upgrade process is a lousy idea and we need a better > >> solution. But let's fix one thing at a time. > > > Agreed on both counts ... but ... does this mean that we need a > > different program for programmable tasks as opposed to interactive > > ones? Dealing with standalone backends *is* a pain, that's for sure. > > So we fix the interface presented by standalone mode to be less insane. > That way, we can *reduce* the net amount of cruft in the system, rather > than adding more as all these proposals do. +1 on that general idea, but who is going to do the work? -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Wed, Jun 15, 2011 at 1:13 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Robert Haas's message of mié jun 15 12:51:29 -0400 2011: >> On Wed, Jun 15, 2011 at 12:12 PM, Alvaro Herrera >> <alvherre@commandprompt.com> wrote: > >> > Agreed on both counts ... but ... does this mean that we need a >> > different program for programmable tasks as opposed to interactive >> > ones? Dealing with standalone backends *is* a pain, that's for sure. >> >> I'm not sure exactly what is needed here - what programmable tasks are >> you thinking of, other than pg_upgrade? > > Well, something to use on shell scripts and the like first and foremost; > see > http://petereisentraut.blogspot.com/2010/03/running-sql-scripts-with-psql.html I don't think there's anything wrong with using psql for running scripts. It might need some work and maybe some better flags, but I don't think we need to throw it out wholesale. What we do need for pg_upgrade is to build more of the logic into either pg_upgrade itself or the server, so it's not spawning external programs right and left. It might help to "library"-ify some of the functionality that's being used so that it can be invoked via a function call rather than a shell exec. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Alvaro Herrera wrote: > Excerpts from Tom Lane's message of mié jun 15 12:52:30 -0400 2011: > > Alvaro Herrera <alvherre@commandprompt.com> writes: > > > Excerpts from Robert Haas's message of mié jun 15 08:45:21 -0400 2011: > > >> As a separate issue, I tend to agree with Tom that using psql as part > > >> of the pg_upgrade process is a lousy idea and we need a better > > >> solution. But let's fix one thing at a time. > > > > > Agreed on both counts ... but ... does this mean that we need a > > > different program for programmable tasks as opposed to interactive > > > ones? Dealing with standalone backends *is* a pain, that's for sure. > > > > So we fix the interface presented by standalone mode to be less insane. > > That way, we can *reduce* the net amount of cruft in the system, rather > > than adding more as all these proposals do. > > +1 on that general idea, but who is going to do the work? And you are going to backpatch all this? I don't find this promising at all. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Robert Haas wrote: > > Also, a standalone backend does not have libpq either so how do you get > > values into application variables? ?Parse the text output? ?That seems > > like a much larger kludge. > > Maybe we could do something like this. > > 1. pg_upgrade invokes the postmaster with --binary-upgrade=<port>:<password> > > 2. postmaster starts up into multi-user mode, but it does not start > autovacuum and ignores pg_hba.conf, listen_addresses, and port. > Instead it listens only on the localhost interface on the designated > port (perhaps the port can be a filename on systems that support UNIX > sockets, and it can listen only on a UNIX socket at that location > instead). It refuses all connections except for those that attempt to > log in with binary_upgrade as the user and the given password as the > password. pg_upgrade will randomly generate a password (like > C51622FA-7513-4300-A4B7-6423769276F8) and port number at the start of > each run, and use that for all connections to the postmaster. I now believe we are overthinking all this. pg_upgrade has always supported specification of a port number. Why not just tell users to specify an unused port number > 1023, and not to use the default value? Both old and new clusters will happily run on any specified port number during the upgrade. This allows the lockout to work for both old and new clusters, which is better than enhancing -b because that will only be for > 9.1 servers. This requires no new backend code. We could even _require_ the port number to be specified in pg_upgrade. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce@momjian.us> wrote: > This requires no new backend code. We could even _require_ the port > number to be specified in pg_upgrade. +1... That seems to have lots of nice properties. -- When confronted by a difficult problem, solve it by reducing it to the question, "How would the Lone Ranger handle this?"
Christopher Browne <cbbrowne@gmail.com> writes: > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce@momjian.us> wrote: >> [ just recommend using a different port number during pg_upgrade ] > +1... That seems to have lots of nice properties. Yeah, that seems like an appropriate expenditure of effort. It's surely not bulletproof, since someone could intentionally connect to the actual port number, but getting to bulletproof is a lot more work than anyone seems to want to do right now. (And, as Bruce pointed out, no complete solution would be back-patchable anyway.) regards, tom lane
On ons, 2011-06-15 at 13:35 -0400, Bruce Momjian wrote: > I now believe we are overthinking all this. pg_upgrade has always > supported specification of a port number. Why not just tell users to > specify an unused port number > 1023, and not to use the default > value? Both old and new clusters will happily run on any specified > port number during the upgrade. This allows the lockout to work for > both old and new clusters, which is better than enhancing -b because > that will only be for > 9.1 servers. On non-Windows servers you could get this even safer by disabling the TCP/IP socket altogether, and placing the Unix-domain socket in a private temporary directory. The "port" wouldn't actually matter then.
Tom Lane wrote: > Christopher Browne <cbbrowne@gmail.com> writes: > > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> [ just recommend using a different port number during pg_upgrade ] > > > +1... That seems to have lots of nice properties. > > Yeah, that seems like an appropriate expenditure of effort. It's surely > not bulletproof, since someone could intentionally connect to the actual > port number, but getting to bulletproof is a lot more work than anyone > seems to want to do right now. (And, as Bruce pointed out, no complete > solution would be back-patchable anyway.) OK, let me work on that. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Peter Eisentraut wrote: > On ons, 2011-06-15 at 13:35 -0400, Bruce Momjian wrote: > > I now believe we are overthinking all this. pg_upgrade has always > > supported specification of a port number. Why not just tell users to > > specify an unused port number > 1023, and not to use the default > > value? Both old and new clusters will happily run on any specified > > port number during the upgrade. This allows the lockout to work for > > both old and new clusters, which is better than enhancing -b because > > that will only be for > 9.1 servers. > > On non-Windows servers you could get this even safer by disabling the > TCP/IP socket altogether, and placing the Unix-domain socket in a > private temporary directory. The "port" wouldn't actually matter then. Yes, it would be nice to just create the socket in the current directory. The fact it doesn't work on Windows would cause our docs to have to differ for Windows, which seems unfortunate. -- 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: > Peter Eisentraut wrote: >> On non-Windows servers you could get this even safer by disabling the >> TCP/IP socket altogether, and placing the Unix-domain socket in a >> private temporary directory. The "port" wouldn't actually matter then. > Yes, it would be nice to just create the socket in the current > directory. The fact it doesn't work on Windows would cause our docs to > have to differ for Windows, which seems unfortunate. It still wouldn't be bulletproof against someone running as the postgres user, so probably not worth the trouble. regards, tom lane
Tom Lane wrote: > Christopher Browne <cbbrowne@gmail.com> writes: > > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> [ just recommend using a different port number during pg_upgrade ] > > > +1... That seems to have lots of nice properties. > > Yeah, that seems like an appropriate expenditure of effort. It's surely > not bulletproof, since someone could intentionally connect to the actual > port number, but getting to bulletproof is a lot more work than anyone > seems to want to do right now. (And, as Bruce pointed out, no complete > solution would be back-patchable anyway.) I have researched this and need feedback. Initially I wanted to use a single -p port flag to be used by the old and new clusters. However, pg_upgrade allows --check mode while the old server is running, so we need to allow you to use the current old postmaster port number and a different port number to test the new server. That kills the idea of using a single -p flag, so -p and -P are needed. So, do we allow -p and -P to default to DEF_PORT or PGPORT? For the live server check, that would be nice, but for the other cases we probably need a different port number. This does mean that for the most common use case they will be specifying the same port number for -p and -P, except for a live check. I am guessing we don't want any port number defaults. People are going to think it is odd to have to supply the same port number for -p and -P. We could allow -P to default to -p when not doing a check, but that seems confusing. Do we want -P to only be used in --check mode? That seems confusing too -- that would mean -p is the old server in --check mode, and the old and new server in non-check mode. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce, * Bruce Momjian (bruce@momjian.us) wrote: > I have researched this and need feedback. In general, I like the whole idea of using random/special ports for the duration of the upgrade. I agree that we need to keep the ability to check the existing clusters. My gut feeling is this: keep the existing port options just as they are, so --check works just fine, etc. Use *only* long-options for the "ports to use during the actual upgrade" and discourage their use- we want people to let a random couple of ports be used during the upgrade to minimize the risk of someone connecting to one of the systems. Obvioulsy, there may be special cases where that's not an option, but I don't think we need to make it easy nor do I think we need to have a short option for it. Thanks, Stephen
Stephen Frost wrote: -- Start of PGP signed section. > Bruce, > > * Bruce Momjian (bruce@momjian.us) wrote: > > I have researched this and need feedback. > > In general, I like the whole idea of using random/special ports for the > duration of the upgrade. I agree that we need to keep the ability to > check the existing clusters. My gut feeling is this: keep the existing > port options just as they are, so --check works just fine, etc. Use > *only* long-options for the "ports to use during the actual upgrade" and > discourage their use- we want people to let a random couple of ports be > used during the upgrade to minimize the risk of someone connecting to > one of the systems. Obvioulsy, there may be special cases where that's > not an option, but I don't think we need to make it easy nor do I think > we need to have a short option for it. Having long options mean different than short options seems very confusing. -- 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) wrote: > Having long options mean different than short options seems very > confusing. Err, that wasn't what I was proposing.. Just having: --old-port-during-upgrade and similar would have to be used if they want to specify the ports to be used during the upgrade proces... We just wouldn't have a short-option for that option, since we discourage it.. Thanks, Stephen
Stephen Frost wrote: -- Start of PGP signed section. > * Bruce Momjian (bruce@momjian.us) wrote: > > Having long options mean different than short options seems very > > confusing. > > Err, that wasn't what I was proposing.. Just having: > --old-port-during-upgrade > > and similar would have to be used if they want to specify the ports to > be used during the upgrade proces... > > We just wouldn't have a short-option for that option, since we > discourage it.. I think that is going to be very hard to document --- seems easier to just use -p and -P always. -- 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: > I have researched this and need feedback. Initially I wanted to use a > single -p port flag to be used by the old and new clusters. However, > pg_upgrade allows --check mode while the old server is running, so we > need to allow you to use the current old postmaster port number and a > different port number to test the new server. That kills the idea of > using a single -p flag, so -p and -P are needed. I am confused as to the point of allowing checks to be done on a "live" server. Presumably, whatever you are checking could be invalidated as soon as you turn your back, so the checks have to be done again anyway as soon as you shut the old server down. So while there might be a use-case for a "check" mode against the existing server, there isn't any need to combine that with checking the new server in the same run. Furthermore, ISTM that there isn't any need to be running both servers at once --- and, indeed, in entirely-plausible configurations you won't be *able* to do that because of SHMMAX; so pg_upgrade must not depend on being able to do so. So on the whole I don't see the reason why two port numbers would be needed. Just run the two servers serially on a single nondefault port number. regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I have researched this and need feedback. Initially I wanted to use a > > single -p port flag to be used by the old and new clusters. However, > > pg_upgrade allows --check mode while the old server is running, so we > > need to allow you to use the current old postmaster port number and a > > different port number to test the new server. That kills the idea of > > using a single -p flag, so -p and -P are needed. > > I am confused as to the point of allowing checks to be done on a "live" > server. Presumably, whatever you are checking could be invalidated as > soon as you turn your back, so the checks have to be done again anyway > as soon as you shut the old server down. So while there might be a > use-case for a "check" mode against the existing server, there isn't any > need to combine that with checking the new server in the same run. Right, we will re-check at the time they do the actual upgrade. This was requested so people can prepare for the real upgrade without having to stop their live server. You do need to check the old and new servers in the same pg_upgrade run to make sure their values match, like pg_controldata. This was particularly useful for the 8.3 to 8.4 upgrades where we checked for imcompatible indexes, etc, and is useful for checking that all the contrib shared objects are in place in the new server, etc. I guess we could check many of them without starting the new server, but there are some that need it, so we just start it and do a full check. > Furthermore, ISTM that there isn't any need to be running both servers > at once --- and, indeed, in entirely-plausible configurations you won't > be *able* to do that because of SHMMAX; so pg_upgrade must not depend on > being able to do so. Again, this happens with --check on a live server, as outlined above. > So on the whole I don't see the reason why two port numbers would be > needed. Just run the two servers serially on a single nondefault port > number. See above. Requiring old and new port numbers, and the documentation wasn't as bad as I thought. Patch attached for review. -- 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 8153e30..40f31ad *** a/contrib/pg_upgrade/option.c --- b/contrib/pg_upgrade/option.c *************** parseCommandLine(int argc, char *argv[]) *** 60,67 **** os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; ! new_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ --- 60,67 ---- os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = atoi(getenv("OLDPGPORT"); ! new_cluster.port = atoi(getenv("NEWPGPORT"); os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ *************** parseCommandLine(int argc, char *argv[]) *** 214,219 **** --- 214,223 ---- validateDirectoryOption(&new_cluster.pgdata, "NEWDATADIR", "-D", "new cluster data resides"); + if (old_cluster.port == 0 || new_cluster.port == 0) + pg_log(PG_FATAL, "You must specify both old and new port numbers\n" + "They can be the same, except when running check on an active old server\n"); + get_pkglibdirs(); } diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index a505707..c56165f *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *************** *** 105,111 **** <varlistentry> <term><option>-k</option></term> <term><option>--link</option></term> ! <listitem><para>link instead of copying files to new cluster</para></listitem> </varlistentry> <varlistentry> --- 105,111 ---- <varlistentry> <term><option>-k</option></term> <term><option>--link</option></term> ! <listitem><para>use hard links instead of copying files to the new cluster</para></listitem> </varlistentry> <varlistentry> *************** *** 117,131 **** <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 ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> ! <listitem><para>the new cluster port number; environment ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> --- 117,131 ---- <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 variable ! <envar>OLDPGPORT</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> ! <listitem><para>the new cluster port number; environment variable ! <envar>NEWPGPORT</></para></listitem> </varlistentry> <varlistentry> *************** mv /usr/local/pgsql /usr/local/pgsql.old *** 201,209 **** <title>Install the new PostgreSQL binaries</title> <para> ! Install the new server's binaries and support files. You can use the ! same port numbers for both clusters, typically 5432, because the old and ! new clusters will not be running at the same time. </para> <para> --- 201,207 ---- <title>Install the new PostgreSQL binaries</title> <para> ! Install the new server's binaries and support files. </para> <para> *************** gmake prefix=/usr/local/pgsql.new instal *** 256,263 **** so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. Also make sure ! pg_upgrade is the only program that can connect to the clusters. </para> </step> --- 254,260 ---- so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. </para> </step> *************** NET STOP pgsql-8.3 (<productname>Postgr *** 293,303 **** <para> Always run the <application>pg_upgrade</> binary of the new server, not the old one. <application>pg_upgrade</> requires the specification of the old and new cluster's ! data and executable (<filename>bin</>) directories. You can also specify separate ! user and port values, and whether you want the data linked instead of ! copied (the default). If you use linking, the upgrade will be much ! faster (hard link data files rather than copying them), but you ! will no longer be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the same file system. See <literal>pg_upgrade --help</> for a full list of options. --- 290,312 ---- <para> Always run the <application>pg_upgrade</> binary of the new server, not the old one. <application>pg_upgrade</> requires the specification of the old and new cluster's ! data and executable (<filename>bin</>) directories, and their port ! numbers. When checking a running old server, specify the current ! old server's port number and specify an unused port number greater ! than 1023 for the new cluster. Specifying an unused port number ! helps prevent users from accidentally connecting to the server ! during the check. For actual upgrades, specify a similar unused ! port number for the old and new servers; you can use the same ! port number for both because the old and new servers never ! run at the same time during an upgrade. Again, use an unused ! port number to avoid accidental user connections. ! </para> ! ! <para> ! You can also specify a user name and whether you want the data ! linked instead of copied (the default). If you use link mode, ! the upgrade will be much faster (no file copying), but you ! will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the same file system. See <literal>pg_upgrade --help</> for a full list of options. *************** pg_upgrade.exe *** 320,325 **** --- 329,336 ---- --new-datadir "C:/Program Files/PostgreSQL/9.0/data" --old-bindir "C:/Program Files/PostgreSQL/8.4/bin" --new-bindir "C:/Program Files/PostgreSQL/9.0/bin" + --old-port 5723 + --new-port 5723 </programlisting> Once started, <command>pg_upgrade</> will verify the two clusters are compatible
On Wed, Jun 15, 2011 at 09:14:16PM -0400, Stephen Frost wrote: > Bruce, > > * Bruce Momjian (bruce@momjian.us) wrote: > > I have researched this and need feedback. > > In general, I like the whole idea of using random/special ports for the > duration of the upgrade. I agree that we need to keep the ability to > check the existing clusters. My gut feeling is this: keep the existing > port options just as they are, so --check works just fine, etc. Use > *only* long-options for the "ports to use during the actual upgrade" and > discourage their use- we want people to let a random couple of ports be > used during the upgrade to minimize the risk of someone connecting to > one of the systems. Obvioulsy, there may be special cases where that's > not an option, but I don't think we need to make it easy nor do I think > we need to have a short option for it. As an operations guy, the idea of an upgrade using a random, non-repeatable port selection gives me the hebejeebees. Mr. Murphy will com knocking, sooner or later, with the server picking a port that just happens to be available right now, because it's service is restarting, or is under inet control. Ross -- Ross Reedstrom, Ph.D. reedstrm@rice.edu Systems Engineer & Admin, Research Scientist phone: 713-348-6166 Connexions http://cnx.org fax: 713-348-3665 Rice University MS-375, Houston, TX 77005 GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE
"Ross J. Reedstrom" <reedstrm@rice.edu> writes: > As an operations guy, the idea of an upgrade using a random, > non-repeatable port selection gives me the hebejeebees. Yeah, I agree. The latest version of the patch doesn't appear to have any random component to it, though --- it just expects the user to provide port numbers as switches. regards, tom lane
* Bruce Momjian (bruce@momjian.us) wrote: > Right, we will re-check at the time they do the actual upgrade. This > was requested so people can prepare for the real upgrade without having > to stop their live server. Exactly. A very good thing to have, and something which I needed and would have been very upset to not have. My case was PostGIS being in the old cluster and not available in the new cluster. I had to fix that before moving forward, but I'm glad that didn't require shutting down the old cluster to find out. Thanks, Stephen
Tom Lane wrote: > "Ross J. Reedstrom" <reedstrm@rice.edu> writes: > > As an operations guy, the idea of an upgrade using a random, > > non-repeatable port selection gives me the hebejeebees. > > Yeah, I agree. The latest version of the patch doesn't appear to have > any random component to it, though --- it just expects the user to > provide port numbers as switches. Oh, you wanted pg_upgrade to pick a random port number? I can do that, but how would it check to see it is unused? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Thu, Jun 16, 2011 at 09:48:12AM -0400, Bruce Momjian wrote: > Tom Lane wrote: > > "Ross J. Reedstrom" <reedstrm@rice.edu> writes: > > > As an operations guy, the idea of an upgrade using a random, > > > non-repeatable port selection gives me the hebejeebees. > > > > Yeah, I agree. The latest version of the patch doesn't appear to have > > any random component to it, though --- it just expects the user to > > provide port numbers as switches. > > Oh, you wanted pg_upgrade to pick a random port number? I can do that, > but how would it check to see it is unused? Oh, no! Lost in translation - randomness in this context would be bad. Heebee-jeebees (usual spelling, I guess) "(idiom) used to describe a feeling of anxiety, apprehension, depression or illness" http://en.wikipedia.org/wiki/Heebie-jeebies Ross -- Ross Reedstrom, Ph.D. reedstrm@rice.edu Systems Engineer & Admin, Research Scientist phone: 713-348-6166 Connexions http://cnx.org fax: 713-348-3665 Rice University MS-375, Houston, TX 77005 GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE
On Thu, Jun 16, 2011 at 1:48 PM, Bruce Momjian <bruce@momjian.us> wrote: > Tom Lane wrote: >> "Ross J. Reedstrom" <reedstrm@rice.edu> writes: >> > As an operations guy, the idea of an upgrade using a random, >> > non-repeatable port selection gives me the hebejeebees. >> >> Yeah, I agree. The latest version of the patch doesn't appear to have >> any random component to it, though --- it just expects the user to >> provide port numbers as switches. > > Oh, you wanted pg_upgrade to pick a random port number? I can do that, > but how would it check to see it is unused? If no port is specified, that *might* be a reasonable behavior, but it certainly throws in a dose of the wrong sort of nondeterminism, hence heebie-jeebies... -- When confronted by a difficult problem, solve it by reducing it to the question, "How would the Lone Ranger handle this?"
On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian <bruce@momjian.us> wrote: > I now believe we are overthinking all this. pg_upgrade has always > supported specification of a port number. Why not just tell users to > specify an unused port number > 1023, and not to use the default value? 1. Because it shouldn't be the user's problem to figure out a good choice of port number. 2. Because we also really ought to be ignoring the contents of pg_hba.conf during an upgrade, and instead have some mechanism that allows pg_upgrade to be sure of getting in (without creating a security hole in the process). I agree that back-patching these changes wouldn't be a wonderful thing, but we are going to do a lot more releases that have pg_upgrade in them in the future than we've already done in the past. It's not a bad thing to try to start improving on the basic mechanism, even if takes a while for versions that support that mechanism to become commonplace. Limiting what we're willing to do the server to improve the pg_upgrade experience in the future to what we're willing to back-patch is not going to be a winning strategy. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian <bruce@momjian.us> wrote: > > I now believe we are overthinking all this. ?pg_upgrade has always > > supported specification of a port number. ?Why not just tell users to > > specify an unused port number > 1023, and not to use the default value? > > 1. Because it shouldn't be the user's problem to figure out a good > choice of port number. > > 2. Because we also really ought to be ignoring the contents of > pg_hba.conf during an upgrade, and instead have some mechanism that > allows pg_upgrade to be sure of getting in (without creating a > security hole in the process). > > I agree that back-patching these changes wouldn't be a wonderful > thing, but we are going to do a lot more releases that have pg_upgrade > in them in the future than we've already done in the past. It's not a > bad thing to try to start improving on the basic mechanism, even if > takes a while for versions that support that mechanism to become > commonplace. Limiting what we're willing to do the server to improve > the pg_upgrade experience in the future to what we're willing to > back-patch is not going to be a winning strategy. OK, well, we have three possible directions: 1. Just document that people should use -p and -P on unused ports to avoid user connections 2. Do #1 and also require -p and -P to be used (no defaults) 3. Have pg_upgrade default to use a typically unused port number, e.g. 25432 (on Unix, it might only be using unix domain sockets anyway) 4. Require appname to match 'binary-upgrade' when in -b (binary-upgrade) mode 5. Disable TCP when in -b mode on Unix (not possible on Windows) We can pick different options for 9.0, 9.1, and 9.2. (For PG 9.0 probably only #1 is appropriate.) FYI, pg_upgrade 9.1 will already lock out non-super users, but that doesn't lock out users from old pre-9.1 servers. So, these are all only a few lines of code --- we just need to figure out what we want. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Thu, Jun 16, 2011 at 5:16 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > I now believe we are overthinking all this. ?pg_upgrade has always >> > supported specification of a port number. ?Why not just tell users to >> > specify an unused port number > 1023, and not to use the default value? >> >> 1. Because it shouldn't be the user's problem to figure out a good >> choice of port number. >> >> 2. Because we also really ought to be ignoring the contents of >> pg_hba.conf during an upgrade, and instead have some mechanism that >> allows pg_upgrade to be sure of getting in (without creating a >> security hole in the process). >> >> I agree that back-patching these changes wouldn't be a wonderful >> thing, but we are going to do a lot more releases that have pg_upgrade >> in them in the future than we've already done in the past. It's not a >> bad thing to try to start improving on the basic mechanism, even if >> takes a while for versions that support that mechanism to become >> commonplace. Limiting what we're willing to do the server to improve >> the pg_upgrade experience in the future to what we're willing to >> back-patch is not going to be a winning strategy. > > OK, well, we have three possible directions: > > 1. Just document that people should use -p and -P on unused ports to > avoid user connections > > 2. Do #1 and also require -p and -P to be used (no defaults) > > 3. Have pg_upgrade default to use a typically unused port number, e.g. > 25432 (on Unix, it might only be using unix domain sockets anyway) > > 4. Require appname to match 'binary-upgrade' when in -b (binary-upgrade) > mode > > 5. Disable TCP when in -b mode on Unix (not possible on Windows) > > We can pick different options for 9.0, 9.1, and 9.2. (For PG 9.0 > probably only #1 is appropriate.) I don't like any of these options as well as what I already proposed. I proposed a complicated approach that actually fixes the problem for real; you're proposing a whole bunch of simpler approaches all of which have pretty obvious holes. We already have something that only sorta works; replacing it with a different system that only sorta works is not going to be a great leap forward. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0 > > probably only #1 is appropriate.) > > I don't like any of these options as well as what I already proposed. > I proposed a complicated approach that actually fixes the problem for > real; you're proposing a whole bunch of simpler approaches all of > which have pretty obvious holes. We already have something that only > sorta works; replacing it with a different system that only sorta > works is not going to be a great leap forward. What is your proposal? Write a password into a file that is read by the postmaster on startup and used for connections? That would remove the "modify pg_hba.conf to 'trust'" step, but again only for new servers. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Thu, Jun 16, 2011 at 11:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0 >> > probably only #1 is appropriate.) >> >> I don't like any of these options as well as what I already proposed. >> I proposed a complicated approach that actually fixes the problem for >> real; you're proposing a whole bunch of simpler approaches all of >> which have pretty obvious holes. We already have something that only >> sorta works; replacing it with a different system that only sorta >> works is not going to be a great leap forward. > > What is your proposal? Write a password into a file that is read by the > postmaster on startup and used for connections? That would remove the > "modify pg_hba.conf to 'trust'" step, but again only for new servers. Yeah, as noted upthread, I'd probably create a binary_upgrade.conf that works like recovery.conf, if it were me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Thu, Jun 16, 2011 at 11:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Robert Haas wrote: > >> > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0 > >> > probably only #1 is appropriate.) > >> > >> I don't like any of these options as well as what I already proposed. > >> I proposed a complicated approach that actually fixes the problem for > >> real; you're proposing a whole bunch of simpler approaches all of > >> which have pretty obvious holes. ?We already have something that only > >> sorta works; replacing it with a different system that only sorta > >> works is not going to be a great leap forward. > > > > What is your proposal? ?Write a password into a file that is read by the > > postmaster on startup and used for connections? ?That would remove the > > "modify pg_hba.conf to 'trust'" step, but again only for new servers. > > Yeah, as noted upthread, I'd probably create a binary_upgrade.conf > that works like recovery.conf, if it were me. Well, I know exactly where the data directories are. We will still have a problem for anyone upgrading from pre-9.2. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > Robert Haas wrote: > > On Thu, Jun 16, 2011 at 11:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > > > Robert Haas wrote: > > >> > We can pick different options for 9.0, 9.1, and 9.2. ?(For PG 9.0 > > >> > probably only #1 is appropriate.) > > >> > > >> I don't like any of these options as well as what I already proposed. > > >> I proposed a complicated approach that actually fixes the problem for > > >> real; you're proposing a whole bunch of simpler approaches all of > > >> which have pretty obvious holes. ?We already have something that only > > >> sorta works; replacing it with a different system that only sorta > > >> works is not going to be a great leap forward. > > > > > > What is your proposal? ?Write a password into a file that is read by the > > > postmaster on startup and used for connections? ?That would remove the > > > "modify pg_hba.conf to 'trust'" step, but again only for new servers. > > > > Yeah, as noted upthread, I'd probably create a binary_upgrade.conf > > that works like recovery.conf, if it were me. > > Well, I know exactly where the data directories are. We will still have > a problem for anyone upgrading from pre-9.2. We could go with the idea of documenting the suggestion of using unused ports in pre-9.2 and use that file for 9.2. We would still have to mention the ports idea in 9.2 as well because of people upgrading from pre-9.2. We can have that file be read only in -b binary-upgrade mode so there is little risk if the file accidentally isn't deleted. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On ons, 2011-06-15 at 17:50 -0400, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Peter Eisentraut wrote: > >> On non-Windows servers you could get this even safer by disabling the > >> TCP/IP socket altogether, and placing the Unix-domain socket in a > >> private temporary directory. The "port" wouldn't actually matter then. > > > Yes, it would be nice to just create the socket in the current > > directory. The fact it doesn't work on Windows would cause our docs to > > have to differ for Windows, which seems unfortunate. > > It still wouldn't be bulletproof against someone running as the postgres > user, so probably not worth the trouble. But the postgres user would normally be the DBA itself, so it'd be his own fault. I don't see how you can easily make any process safe from interference by the same user account.
Peter Eisentraut <peter_e@gmx.net> writes: > On ons, 2011-06-15 at 17:50 -0400, Tom Lane wrote: >> Bruce Momjian <bruce@momjian.us> writes: >>> Peter Eisentraut wrote: >>>> On non-Windows servers you could get this even safer by disabling the >>>> TCP/IP socket altogether, and placing the Unix-domain socket in a >>>> private temporary directory. The "port" wouldn't actually matter then. >>> Yes, it would be nice to just create the socket in the current >>> directory. The fact it doesn't work on Windows would cause our docs to >>> have to differ for Windows, which seems unfortunate. >> It still wouldn't be bulletproof against someone running as the postgres >> user, so probably not worth the trouble. > But the postgres user would normally be the DBA itself, so it'd be his > own fault. I don't see how you can easily make any process safe from > interference by the same user account. Well, the point here is that it's not bulletproof, it's just making it incrementally harder to connect accidentally. Given that Windows wouldn't be covered, I don't see that it's worth the trouble compared to just switching to a nondefault port number. (Am I wrong to think that Windows users are more likely to mess up here?) regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > On ons, 2011-06-15 at 17:50 -0400, Tom Lane wrote: > >> Bruce Momjian <bruce@momjian.us> writes: > >>> Peter Eisentraut wrote: > >>>> On non-Windows servers you could get this even safer by disabling the > >>>> TCP/IP socket altogether, and placing the Unix-domain socket in a > >>>> private temporary directory. The "port" wouldn't actually matter then. > > >>> Yes, it would be nice to just create the socket in the current > >>> directory. The fact it doesn't work on Windows would cause our docs to > >>> have to differ for Windows, which seems unfortunate. > > >> It still wouldn't be bulletproof against someone running as the postgres > >> user, so probably not worth the trouble. > > > But the postgres user would normally be the DBA itself, so it'd be his > > own fault. I don't see how you can easily make any process safe from > > interference by the same user account. > > Well, the point here is that it's not bulletproof, it's just making it > incrementally harder to connect accidentally. Given that Windows > wouldn't be covered, I don't see that it's worth the trouble compared to > just switching to a nondefault port number. (Am I wrong to think that > Windows users are more likely to mess up here?) Windows is not covered if we shut off TCP and just use unix domain sockets --- that is the only Windows-specific part I know. Windows does work with the non-default port, and with writing the password to a file. (FYI, I think we would need to use PGPASSWORD for the password file option, and we don't recommend PGPASSWORD use in our docs.) PG 9.1 already has code to lock out non-super users, but only for 9.1+ servers --- writing a password to a file would have the same only 9.2+ restriction. Non-default port numbers would work for all PG versions because that is tied to the pg_upgrade binary. Again, everything is easy to do --- we just have to decide. I hoped my listing 5 items would unleash a flood of votes --- no such luck. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 06/17/2011 06:59 PM, Bruce Momjian wrote: > > (FYI, I think we would need to use PGPASSWORD for the password file > option, and we don't recommend PGPASSWORD use in our docs.) > er what? did you mean PGPASSFILE? cheers andrew
Andrew Dunstan wrote: > > > On 06/17/2011 06:59 PM, Bruce Momjian wrote: > > > > (FYI, I think we would need to use PGPASSWORD for the password file > > option, and we don't recommend PGPASSWORD use in our docs.) > > > > er what? > > did you mean PGPASSFILE? I meant the PGPASSWORD environment variable: <indexterm> <primary><envar>PGPASSWORD</envar></primary> </indexterm> <envar>PGPASSWORD</envar> behaves the same as the <xref linkend="libpq-connect-password"> connection parameter. Use of this environment variable is not recommended for security reasons, as some operating systems allow non-root users to see process environment variables via <application>ps</>; instead consider using the <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">). The only other way to do this is to pass it on the command line, but some options don't allow that (pg_ctl), and PGPASSFILE is going to require me to create a dummy .pgpass password file in a valid format and use that. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > I meant the PGPASSWORD environment variable: > > <indexterm> > <primary><envar>PGPASSWORD</envar></primary> > </indexterm> > <envar>PGPASSWORD</envar> behaves the same as the <xref > linkend="libpq-connect-password"> connection parameter. > Use of this environment variable > is not recommended for security reasons, as some operating systems > allow non-root users to see process environment variables via > <application>ps</>; instead consider using the > <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">). > > The only other way to do this is to pass it on the command line, but > some options don't allow that (pg_ctl), and PGPASSFILE is going to > require me to create a dummy .pgpass password file in a valid format and > use that. One interesting idea would be to write the server password in the PGPASSFILE format, and allow the server and libpq to read the same file by pointing PGPASSFILE at that file. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > Bruce Momjian wrote: > > I meant the PGPASSWORD environment variable: > > > > <indexterm> > > <primary><envar>PGPASSWORD</envar></primary> > > </indexterm> > > <envar>PGPASSWORD</envar> behaves the same as the <xref > > linkend="libpq-connect-password"> connection parameter. > > Use of this environment variable > > is not recommended for security reasons, as some operating systems > > allow non-root users to see process environment variables via > > <application>ps</>; instead consider using the > > <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">). > > > > The only other way to do this is to pass it on the command line, but > > some options don't allow that (pg_ctl), and PGPASSFILE is going to > > require me to create a dummy .pgpass password file in a valid format and > > use that. > > One interesting idea would be to write the server password in the > PGPASSFILE format, and allow the server and libpq to read the same file > by pointing PGPASSFILE at that file. Discussion seems to have ended on this thread without a clear direction. Let me throw out some new ideas. The advantage of writing a password file is that we can avoid the need to modify pg_hba.conf to allow access without supplying a password. The downside is that it will only apply to PG 9.2+ servers, making configuration even more complex because the new and old servers would behave differently. One simple improvement would be to set listen_addresses to '' on Unix and 'localhost' on Windows to limit who can connect. This works on old and new servers. PG 9.1 already allows only super-user connections in -b binary-upgrade mode. No one seemed to like the appname filter but it seemed like a cheap solution. Remember we are not trying to secure the server while in pg_upgrade mode --- only avoid accidental connections. And, again, using a default port number of 25432 will work for old/new servers. -- 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: > Discussion seems to have ended on this thread without a clear direction. I still think the right thing is to just use a non-default port number. That gets 90% of the benefit for 10% of the work of any other approach (except for the ones for which the ratio is even worse). regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Discussion seems to have ended on this thread without a clear direction. > > I still think the right thing is to just use a non-default port number. > That gets 90% of the benefit for 10% of the work of any other approach > (except for the ones for which the ratio is even worse). I agree. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Tom Lane wrote: > Christopher Browne <cbbrowne@gmail.com> writes: > > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> [ just recommend using a different port number during pg_upgrade ] > > > +1... That seems to have lots of nice properties. > > Yeah, that seems like an appropriate expenditure of effort. It's surely > not bulletproof, since someone could intentionally connect to the actual > port number, but getting to bulletproof is a lot more work than anyone > seems to want to do right now. (And, as Bruce pointed out, no complete > solution would be back-patchable anyway.) I have created the following patch which uses 25432 as the default port number for pg_upgrade. It also creates two new environment variables, OLDPGPORT and NEWPGPORT, to control the port values because we don't want to default to PGPORT anymore. This will allow most users to use pg_upgrade without needing to specify port parameters, and will prevent accidental connections. The user does have to specify the port number for live checks, but that was always the case because you have to use different port numbers for old/new in that case. I updated the error message to be clearer about this. The patch is small and might be possible for 9.1, except it changes user-visible behavior, which would suggest it should be in 9.2, plus it doesn't address a serious bug. Comments? I will batckpatch a doc recommendation to use 25432 as a port number for versions of pg_upgrade that don't get this patch. -- 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/check.c b/contrib/pg_upgrade/check.c new file mode 100644 index 1ee2aca..323b5f1 *** a/contrib/pg_upgrade/check.c --- b/contrib/pg_upgrade/check.c *************** output_check_banner(bool *live_check) *** 30,37 **** { *live_check = true; if (old_cluster.port == new_cluster.port) ! pg_log(PG_FATAL, "When checking a live server, " ! "the old and new port numbers must be different.\n"); pg_log(PG_REPORT, "Performing Consistency Checks on Old Live Server\n"); pg_log(PG_REPORT, "------------------------------------------------\n"); } --- 30,37 ---- { *live_check = true; if (old_cluster.port == new_cluster.port) ! pg_log(PG_FATAL, "When checking a live old server, " ! "you must specify the old server's port number.\n"); pg_log(PG_REPORT, "Performing Consistency Checks on Old Live Server\n"); pg_log(PG_REPORT, "------------------------------------------------\n"); } diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c new file mode 100644 index 4401a81..7fbfa8c *** a/contrib/pg_upgrade/option.c --- b/contrib/pg_upgrade/option.c *************** parseCommandLine(int argc, char *argv[]) *** 58,65 **** os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; ! new_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ --- 58,65 ---- os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("OLDPGPORT") ? atoi(getenv("OLDPGPORT")) : DEF_PGUPORT; ! new_cluster.port = getenv("NEWPGPORT") ? atoi(getenv("NEWPGPORT")) : DEF_PGUPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ *************** Options:\n\ *** 231,238 **** -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\ ! -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\ -v, --verbose enable verbose output\n\ -V, --version display version information, then exit\n\ --- 231,238 ---- -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\ ! -p, --old-port=OLDPGPORT old cluster port number (default %d)\n\ ! -P, --new-port=NEWPGPORT new cluster port number (default %d)\n\ -u, --user=NAME clusters superuser (default \"%s\")\n\ -v, --verbose enable verbose output\n\ -V, --version display version information, then exit\n\ diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h new file mode 100644 index 613ddbd..ff52fc2 *** a/contrib/pg_upgrade/pg_upgrade.h --- b/contrib/pg_upgrade/pg_upgrade.h *************** *** 15,20 **** --- 15,26 ---- #include "libpq-fe.h" + /* + * Port 25432 is not known to be used by anyone. 2011-06-23 + * http://www.networksorcery.com/enp/protocol/ip/ports25000.htm + */ + #define DEF_PGUPORT 25432 + /* Allocate for null byte */ #define USER_NAME_SIZE 128 diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index b24c1e7..ae6469e *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *************** *** 118,131 **** <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 ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> <listitem><para>the new cluster port number; environment ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> --- 118,131 ---- <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 ! variable <envar>OLDPGPORT</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> <listitem><para>the new cluster port number; environment ! variable <envar>NEWPGPORT</></para></listitem> </varlistentry> <varlistentry> *************** gmake prefix=/usr/local/pgsql.new instal *** 256,263 **** so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. Also make sure ! pg_upgrade is the only program that can connect to the clusters. </para> </step> --- 256,262 ---- so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. </para> </step> *************** NET STOP pgsql-8.3 (<productname>Postgr *** 303,311 **** copying), but you will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the ! same file system. See <literal>pg_upgrade --help</> for a full ! list of options. ! </para> <para> For Windows users, you must be logged into an administrative account, and --- 302,315 ---- copying), but you will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the ! same file system. ! </para> ! ! <para> ! <application>pg_upgrade</> defaults to running servers on port ! 25432 to avoid unintended client connections. See <literal>pg_upgrade ! --help</> for a full list of options. ! </para> <para> For Windows users, you must be logged into an administrative account, and
On tor, 2011-06-23 at 21:39 -0400, Bruce Momjian wrote: > I have created the following patch which uses 25432 as the default port > number for pg_upgrade. I don't think we should just steal a port from the reserved range. Picking a random port from the private/dynamic range seems more appropriate. > It also creates two new environment variables, > OLDPGPORT and NEWPGPORT, to control the port values because we don't > want to default to PGPORT anymore. I would prefer that all PostgreSQL-related environment variables start with "PG".
On Fri, Jun 24, 2011 at 9:04 AM, Peter Eisentraut <peter_e@gmx.net> wrote: >> It also creates two new environment variables, >> OLDPGPORT and NEWPGPORT, to control the port values because we don't >> want to default to PGPORT anymore. > > I would prefer that all PostgreSQL-related environment variables start > with "PG". +1 -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Peter Eisentraut wrote: > On tor, 2011-06-23 at 21:39 -0400, Bruce Momjian wrote: > > I have created the following patch which uses 25432 as the default port > > number for pg_upgrade. > > I don't think we should just steal a port from the reserved range. > Picking a random port from the private/dynamic range seems more > appropriate. Oh, I didn't know about that. I will use 50432 instead. > > It also creates two new environment variables, > > OLDPGPORT and NEWPGPORT, to control the port values because we don't > > want to default to PGPORT anymore. > > I would prefer that all PostgreSQL-related environment variables start > with "PG". OK, attached. I was also using environment variables for PGDATA and PGBIN do I renamed those too to begin with 'PG'. Patch attached. -- 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/check.c b/contrib/pg_upgrade/check.c new file mode 100644 index 1ee2aca..5c5ce72 *** a/contrib/pg_upgrade/check.c --- b/contrib/pg_upgrade/check.c *************** output_check_banner(bool *live_check) *** 29,34 **** --- 29,37 ---- if (user_opts.check && is_server_running(old_cluster.pgdata)) { *live_check = true; + if (old_cluster.port == DEF_PGUPORT) + pg_log(PG_FATAL, "When checking a live old server, " + "you must specify the old server's port number.\n"); if (old_cluster.port == new_cluster.port) pg_log(PG_FATAL, "When checking a live server, " "the old and new port numbers must be different.\n"); diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c new file mode 100644 index 4401a81..d29aad0 *** a/contrib/pg_upgrade/option.c --- b/contrib/pg_upgrade/option.c *************** parseCommandLine(int argc, char *argv[]) *** 58,65 **** os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; ! new_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ --- 58,65 ---- os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("PGPORTOLD") ? atoi(getenv("PGPORTOLD")) : DEF_PGUPORT; ! new_cluster.port = getenv("PGPORTNEW") ? atoi(getenv("PGPORTNEW")) : DEF_PGUPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ *************** parseCommandLine(int argc, char *argv[]) *** 203,215 **** } /* Get values from env if not already set */ ! check_required_directory(&old_cluster.bindir, "OLDBINDIR", "-b", "old cluster binaries reside"); ! check_required_directory(&new_cluster.bindir, "NEWBINDIR", "-B", "new cluster binaries reside"); ! check_required_directory(&old_cluster.pgdata, "OLDDATADIR", "-d", "old cluster data resides"); ! check_required_directory(&new_cluster.pgdata, "NEWDATADIR", "-D", "new cluster data resides"); } --- 203,215 ---- } /* 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"); } *************** For example:\n\ *** 254,270 **** or\n"), old_cluster.port, new_cluster.port, os_info.user); #ifndef WIN32 printf(_("\ ! $ export OLDDATADIR=oldCluster/data\n\ ! $ export NEWDATADIR=newCluster/data\n\ ! $ export OLDBINDIR=oldCluster/bin\n\ ! $ export NEWBINDIR=newCluster/bin\n\ $ pg_upgrade\n")); #else printf(_("\ ! C:\\> set OLDDATADIR=oldCluster/data\n\ ! C:\\> set NEWDATADIR=newCluster/data\n\ ! C:\\> set OLDBINDIR=oldCluster/bin\n\ ! C:\\> set NEWBINDIR=newCluster/bin\n\ C:\\> pg_upgrade\n")); #endif printf(_("\nReport bugs to <pgsql-bugs@postgresql.org>.\n")); --- 254,270 ---- or\n"), old_cluster.port, new_cluster.port, os_info.user); #ifndef WIN32 printf(_("\ ! $ export PGDATAOLD=oldCluster/data\n\ ! $ export PGDATANEW=newCluster/data\n\ ! $ export PGBINOLD=oldCluster/bin\n\ ! $ export PGBINNEW=newCluster/bin\n\ $ pg_upgrade\n")); #else printf(_("\ ! C:\\> set PGDATAOLD=oldCluster/data\n\ ! C:\\> set PGDATANEW=newCluster/data\n\ ! C:\\> set PGBINOLD=oldCluster/bin\n\ ! C:\\> set PGBINNEW=newCluster/bin\n\ C:\\> pg_upgrade\n")); #endif printf(_("\nReport bugs to <pgsql-bugs@postgresql.org>.\n")); diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h new file mode 100644 index 613ddbd..4729ac3 *** a/contrib/pg_upgrade/pg_upgrade.h --- b/contrib/pg_upgrade/pg_upgrade.h *************** *** 15,20 **** --- 15,23 ---- #include "libpq-fe.h" + /* Use port in the private/dynamic port number range */ + #define DEF_PGUPORT 50432 + /* Allocate for null byte */ #define USER_NAME_SIZE 128 diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index b24c1e7..aa633e2 *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *************** *** 60,73 **** <term><option>-b</option> <replaceable>old_bindir</></term> <term><option>--old-bindir=</option><replaceable>old_bindir</></term> <listitem><para>the old cluster executable directory; ! environment variable <envar>OLDBINDIR</></para></listitem> </varlistentry> <varlistentry> <term><option>-B</option> <replaceable>new_bindir</></term> <term><option>--new-bindir=</option><replaceable>new_bindir</></term> <listitem><para>the new cluster executable directory; ! environment variable <envar>NEWBINDIR</></para></listitem> </varlistentry> <varlistentry> --- 60,73 ---- <term><option>-b</option> <replaceable>old_bindir</></term> <term><option>--old-bindir=</option><replaceable>old_bindir</></term> <listitem><para>the old cluster executable directory; ! environment variable <envar>PGBINOLD</></para></listitem> </varlistentry> <varlistentry> <term><option>-B</option> <replaceable>new_bindir</></term> <term><option>--new-bindir=</option><replaceable>new_bindir</></term> <listitem><para>the new cluster executable directory; ! environment variable <envar>PGBINNEW</></para></listitem> </varlistentry> <varlistentry> *************** *** 80,93 **** <term><option>-d</option> <replaceable>old_datadir</></term> <term><option>--old-datadir=</option><replaceable>old_datadir</></term> <listitem><para>the old cluster data directory; environment ! variable <envar>OLDDATADIR</></para></listitem> </varlistentry> <varlistentry> <term><option>-D</option> <replaceable>new_datadir</></term> <term><option>--new-datadir=</option><replaceable>new_datadir</></term> <listitem><para>the new cluster data directory; environment ! variable <envar>NEWDATADIR</></para></listitem> </varlistentry> <varlistentry> --- 80,93 ---- <term><option>-d</option> <replaceable>old_datadir</></term> <term><option>--old-datadir=</option><replaceable>old_datadir</></term> <listitem><para>the old cluster data directory; environment ! variable <envar>PGDATAOLD</></para></listitem> </varlistentry> <varlistentry> <term><option>-D</option> <replaceable>new_datadir</></term> <term><option>--new-datadir=</option><replaceable>new_datadir</></term> <listitem><para>the new cluster data directory; environment ! variable <envar>PGDATANEW</></para></listitem> </varlistentry> <varlistentry> *************** *** 118,131 **** <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 ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> <listitem><para>the new cluster port number; environment ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> --- 118,131 ---- <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 ! variable <envar>PGPORTOLD</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> <listitem><para>the new cluster port number; environment ! variable <envar>PGPORTNEW</></para></listitem> </varlistentry> <varlistentry> *************** gmake prefix=/usr/local/pgsql.new instal *** 256,263 **** so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. Also make sure ! pg_upgrade is the only program that can connect to the clusters. </para> </step> --- 256,262 ---- so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. </para> </step> *************** NET STOP pgsql-8.3 (<productname>Postgr *** 303,311 **** copying), but you will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the ! same file system. See <literal>pg_upgrade --help</> for a full ! list of options. ! </para> <para> For Windows users, you must be logged into an administrative account, and --- 302,315 ---- copying), but you will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the ! same file system. ! </para> ! ! <para> ! <application>pg_upgrade</> defaults to running servers on port ! 50432 to avoid unintended client connections. See <literal>pg_upgrade ! --help</> for a full list of options. ! </para> <para> For Windows users, you must be logged into an administrative account, and
On fre, 2011-06-24 at 16:34 -0400, Bruce Momjian wrote: > > > It also creates two new environment variables, > > > OLDPGPORT and NEWPGPORT, to control the port values because we > don't > > > want to default to PGPORT anymore. > > > > I would prefer that all PostgreSQL-related environment variables > start > > with "PG". > > OK, attached. I was also using environment variables for PGDATA and > PGBIN do I renamed those too to begin with 'PG'. I'm wondering why pg_upgrade needs environment variables at all. It's a one-shot operation. Environment variables are typically used to shared default settings across programs. I don't see how that applies here.
Peter Eisentraut wrote: > On fre, 2011-06-24 at 16:34 -0400, Bruce Momjian wrote: > > > > It also creates two new environment variables, > > > > OLDPGPORT and NEWPGPORT, to control the port values because we > > don't > > > > want to default to PGPORT anymore. > > > > > > I would prefer that all PostgreSQL-related environment variables > > start > > > with "PG". > > > > OK, attached. I was also using environment variables for PGDATA and > > PGBIN do I renamed those too to begin with 'PG'. > > I'm wondering why pg_upgrade needs environment variables at all. It's a > one-shot operation. Environment variables are typically used to shared > default settings across programs. I don't see how that applies here. They were there in the original EnterpriseDB code, and in some cases like PGPORT, we _used_ those environment variables. Also, the command-line can get pretty long so we actually illustrate environment variable use in its --help: For example: pg_upgrade -d oldCluster/data -D newCluster/data -b oldCluster/bin -B newCluster/binor $ export OLDDATADIR=oldCluster/data $ export NEWDATADIR=newCluster/data $ export OLDBINDIR=oldCluster/bin $ export NEWBINDIR=newCluster/bin $ pg_upgrade You want the environment variable support removed? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Fri, Jun 24, 2011 at 7:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > Peter Eisentraut wrote: >> On fre, 2011-06-24 at 16:34 -0400, Bruce Momjian wrote: >> > > > It also creates two new environment variables, >> > > > OLDPGPORT and NEWPGPORT, to control the port values because we >> > don't >> > > > want to default to PGPORT anymore. >> > > >> > > I would prefer that all PostgreSQL-related environment variables >> > start >> > > with "PG". >> > >> > OK, attached. I was also using environment variables for PGDATA and >> > PGBIN do I renamed those too to begin with 'PG'. >> >> I'm wondering why pg_upgrade needs environment variables at all. It's a >> one-shot operation. Environment variables are typically used to shared >> default settings across programs. I don't see how that applies here. > > They were there in the original EnterpriseDB code, and in some cases > like PGPORT, we _used_ those environment variables. Also, the > command-line can get pretty long so we actually illustrate environment > variable use in its --help: > > For example: > pg_upgrade -d oldCluster/data -D newCluster/data -b oldCluster/bin -B newCluster/bin > or > $ export OLDDATADIR=oldCluster/data > $ export NEWDATADIR=newCluster/data > $ export OLDBINDIR=oldCluster/bin > $ export NEWBINDIR=newCluster/bin > $ pg_upgrade > > You want the environment variable support removed? I don't. It's production usefulness is questionable, but it's quite handy for testing IMO. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On fre, 2011-06-24 at 19:47 -0400, Bruce Momjian wrote: > > I'm wondering why pg_upgrade needs environment variables at all. It's a > > one-shot operation. Environment variables are typically used to shared > > default settings across programs. I don't see how that applies here. > > They were there in the original EnterpriseDB code, Well, the original code wasn't the most stellar example of user interface engineering, as we have found out over time. ;-) > and in some cases like PGPORT, we _used_ those environment variables. Right, and I don't mind using *shared* environment variables for common settings, when appropriate. But in some cases, as you remember, we had to work around them or disable them because environment variables essentially leave traps lying around. So now we are about to create even more traps. > Also, the > command-line can get pretty long so we actually illustrate environment > variable use in its --help: > > For example: > pg_upgrade -d oldCluster/data -D newCluster/data -b oldCluster/bin -B newCluster/bin > or > $ export OLDDATADIR=oldCluster/data > $ export NEWDATADIR=newCluster/data > $ export OLDBINDIR=oldCluster/bin > $ export NEWBINDIR=newCluster/bin > $ pg_upgrade I don't see how that makes it better, except that it's somehow prettier perhaps. There are a lot of programs out there with lots of options. I have never seen one that creates one-shot environment variables to make it easier to invoke. In the above case, you create a bunch of traps. If the user abandons the attempt to run pg_upgrade but leaves the shell open, comes back at some other time (or, say, someone else who also logs into the shared postgres account), and runs just "pg_upgrade" for lack of a better idea or forgets an option, a destructive operation might start. Yes, they are stupid and it's their fault and there are other ways to break things, but pg_upgrade is already tricky enough, we don't need to add more hidden ways to break it. (Besides, the above isn't even a portable way to set environment variables. You need to run the assignment and the export separately.) > You want the environment variable support removed? Well, it might be difficult to get consensus on that. I would argue that we don't need to add new ones for a marginal operation like the pg_upgrade check mode. On the other hand, a way to permanently override the new upgrade port you are working on might be useful. It's not clear from the patch how to do that, actually.
Peter Eisentraut wrote: > In the above case, you create a bunch of traps. If the user abandons > the attempt to run pg_upgrade but leaves the shell open, comes back at > some other time (or, say, someone else who also logs into the shared > postgres account), and runs just "pg_upgrade" for lack of a better idea > or forgets an option, a destructive operation might start. Yes, they > are stupid and it's their fault and there are other ways to break > things, but pg_upgrade is already tricky enough, we don't need to add > more hidden ways to break it. > > (Besides, the above isn't even a portable way to set environment > variables. You need to run the assignment and the export separately.) True. > > You want the environment variable support removed? > > Well, it might be difficult to get consensus on that. I would argue > that we don't need to add new ones for a marginal operation like the > pg_upgrade check mode. Well, hard to make any changes without consensus. None of these variables are check-mode only. > On the other hand, a way to permanently override the new upgrade port > you are working on might be useful. It's not clear from the patch how > to do that, actually. That's because the flags to control the port numbers were already there; I only changed pg_upgrade to use new environment variables and changed their defaults to 50234, and no longer use PGPORT so I don't import the runtime port number. I think that is why it seems unclear. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Excerpts from Robert Haas's message of vie jun 24 22:22:55 -0400 2011: > On Fri, Jun 24, 2011 at 7:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > > You want the environment variable support removed? > > I don't. It's production usefulness is questionable, but it's quite > handy for testing IMO. If that's what you want, I think being able to read a file (whose filename you pass with a switch to pg_upgrade) with a bunch of settings is even more convenient. Heck, maybe it's more convenient for the user too. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Sun, Jun 26, 2011 at 10:33 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Robert Haas's message of vie jun 24 22:22:55 -0400 2011: >> On Fri, Jun 24, 2011 at 7:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> > You want the environment variable support removed? >> >> I don't. It's production usefulness is questionable, but it's quite >> handy for testing IMO. > > If that's what you want, I think being able to read a file (whose > filename you pass with a switch to pg_upgrade) with a bunch of settings > is even more convenient. Heck, maybe it's more convenient for the user > too. If someone wants to do the work, I'm all in favor. But I don't feel that we should insist that Bruce do it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Sun, Jun 26, 2011 at 10:33 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > Excerpts from Robert Haas's message of vie jun 24 22:22:55 -0400 2011: > >> On Fri, Jun 24, 2011 at 7:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > > > >> > You want the environment variable support removed? > >> > >> I don't. ?It's production usefulness is questionable, but it's quite > >> handy for testing IMO. > > > > If that's what you want, I think being able to read a file (whose > > filename you pass with a switch to pg_upgrade) with a bunch of settings > > is even more convenient. ?Heck, maybe it's more convenient for the user > > too. > > If someone wants to do the work, I'm all in favor. But I don't feel > that we should insist that Bruce do it. Is there agreement to remove all pg_upgrade-specific environment variables? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Mon, Jun 27, 2011 at 1:39 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Sun, Jun 26, 2011 at 10:33 PM, Alvaro Herrera >> <alvherre@commandprompt.com> wrote: >> > Excerpts from Robert Haas's message of vie jun 24 22:22:55 -0400 2011: >> >> On Fri, Jun 24, 2011 at 7:47 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > >> >> > You want the environment variable support removed? >> >> >> >> I don't. ?It's production usefulness is questionable, but it's quite >> >> handy for testing IMO. >> > >> > If that's what you want, I think being able to read a file (whose >> > filename you pass with a switch to pg_upgrade) with a bunch of settings >> > is even more convenient. ?Heck, maybe it's more convenient for the user >> > too. >> >> If someone wants to do the work, I'm all in favor. But I don't feel >> that we should insist that Bruce do it. > > Is there agreement to remove all pg_upgrade-specific environment > variables? I'm not in favor of that unless we have a workable replacement for them. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
\Robert Haas wrote: > On Mon, Jun 27, 2011 at 1:39 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Robert Haas wrote: > >> On Sun, Jun 26, 2011 at 10:33 PM, Alvaro Herrera > >> <alvherre@commandprompt.com> wrote: > >> > Excerpts from Robert Haas's message of vie jun 24 22:22:55 -0400 2011: > >> >> On Fri, Jun 24, 2011 at 7:47 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> > > >> >> > You want the environment variable support removed? > >> >> > >> >> I don't. ?It's production usefulness is questionable, but it's quite > >> >> handy for testing IMO. > >> > > >> > If that's what you want, I think being able to read a file (whose > >> > filename you pass with a switch to pg_upgrade) with a bunch of settings > >> > is even more convenient. ?Heck, maybe it's more convenient for the user > >> > too. > >> > >> If someone wants to do the work, I'm all in favor. ?But I don't feel > >> that we should insist that Bruce do it. > > > > Is there agreement to remove all pg_upgrade-specific environment > > variables? > > I'm not in favor of that unless we have a workable replacement for them. OK, fair enough. Should I apply my ports patch to Postgres 9.2? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: > OK, fair enough. Should I apply my ports patch to Postgres 9.2? I'm not sure which patch you are referring to. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: > > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? > > I'm not sure which patch you are referring to. This one which makes 50432 the default port. -- 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/check.c b/contrib/pg_upgrade/check.c new file mode 100644 index 1ee2aca..5c5ce72 *** a/contrib/pg_upgrade/check.c --- b/contrib/pg_upgrade/check.c *************** output_check_banner(bool *live_check) *** 29,34 **** --- 29,37 ---- if (user_opts.check && is_server_running(old_cluster.pgdata)) { *live_check = true; + if (old_cluster.port == DEF_PGUPORT) + pg_log(PG_FATAL, "When checking a live old server, " + "you must specify the old server's port number.\n"); if (old_cluster.port == new_cluster.port) pg_log(PG_FATAL, "When checking a live server, " "the old and new port numbers must be different.\n"); diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c new file mode 100644 index 4401a81..d29aad0 *** a/contrib/pg_upgrade/option.c --- b/contrib/pg_upgrade/option.c *************** parseCommandLine(int argc, char *argv[]) *** 58,65 **** os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; ! new_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ --- 58,65 ---- os_info.progname = get_progname(argv[0]); /* Process libpq env. variables; load values here for usage() output */ ! old_cluster.port = getenv("PGPORTOLD") ? atoi(getenv("PGPORTOLD")) : DEF_PGUPORT; ! new_cluster.port = getenv("PGPORTNEW") ? atoi(getenv("PGPORTNEW")) : DEF_PGUPORT; os_user_effective_id = get_user_info(&os_info.user); /* we override just the database user name; we got the OS id above */ *************** parseCommandLine(int argc, char *argv[]) *** 203,215 **** } /* Get values from env if not already set */ ! check_required_directory(&old_cluster.bindir, "OLDBINDIR", "-b", "old cluster binaries reside"); ! check_required_directory(&new_cluster.bindir, "NEWBINDIR", "-B", "new cluster binaries reside"); ! check_required_directory(&old_cluster.pgdata, "OLDDATADIR", "-d", "old cluster data resides"); ! check_required_directory(&new_cluster.pgdata, "NEWDATADIR", "-D", "new cluster data resides"); } --- 203,215 ---- } /* 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"); } *************** For example:\n\ *** 254,270 **** or\n"), old_cluster.port, new_cluster.port, os_info.user); #ifndef WIN32 printf(_("\ ! $ export OLDDATADIR=oldCluster/data\n\ ! $ export NEWDATADIR=newCluster/data\n\ ! $ export OLDBINDIR=oldCluster/bin\n\ ! $ export NEWBINDIR=newCluster/bin\n\ $ pg_upgrade\n")); #else printf(_("\ ! C:\\> set OLDDATADIR=oldCluster/data\n\ ! C:\\> set NEWDATADIR=newCluster/data\n\ ! C:\\> set OLDBINDIR=oldCluster/bin\n\ ! C:\\> set NEWBINDIR=newCluster/bin\n\ C:\\> pg_upgrade\n")); #endif printf(_("\nReport bugs to <pgsql-bugs@postgresql.org>.\n")); --- 254,270 ---- or\n"), old_cluster.port, new_cluster.port, os_info.user); #ifndef WIN32 printf(_("\ ! $ export PGDATAOLD=oldCluster/data\n\ ! $ export PGDATANEW=newCluster/data\n\ ! $ export PGBINOLD=oldCluster/bin\n\ ! $ export PGBINNEW=newCluster/bin\n\ $ pg_upgrade\n")); #else printf(_("\ ! C:\\> set PGDATAOLD=oldCluster/data\n\ ! C:\\> set PGDATANEW=newCluster/data\n\ ! C:\\> set PGBINOLD=oldCluster/bin\n\ ! C:\\> set PGBINNEW=newCluster/bin\n\ C:\\> pg_upgrade\n")); #endif printf(_("\nReport bugs to <pgsql-bugs@postgresql.org>.\n")); diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h new file mode 100644 index 613ddbd..4729ac3 *** a/contrib/pg_upgrade/pg_upgrade.h --- b/contrib/pg_upgrade/pg_upgrade.h *************** *** 15,20 **** --- 15,23 ---- #include "libpq-fe.h" + /* Use port in the private/dynamic port number range */ + #define DEF_PGUPORT 50432 + /* Allocate for null byte */ #define USER_NAME_SIZE 128 diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index b24c1e7..aa633e2 *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *************** *** 60,73 **** <term><option>-b</option> <replaceable>old_bindir</></term> <term><option>--old-bindir=</option><replaceable>old_bindir</></term> <listitem><para>the old cluster executable directory; ! environment variable <envar>OLDBINDIR</></para></listitem> </varlistentry> <varlistentry> <term><option>-B</option> <replaceable>new_bindir</></term> <term><option>--new-bindir=</option><replaceable>new_bindir</></term> <listitem><para>the new cluster executable directory; ! environment variable <envar>NEWBINDIR</></para></listitem> </varlistentry> <varlistentry> --- 60,73 ---- <term><option>-b</option> <replaceable>old_bindir</></term> <term><option>--old-bindir=</option><replaceable>old_bindir</></term> <listitem><para>the old cluster executable directory; ! environment variable <envar>PGBINOLD</></para></listitem> </varlistentry> <varlistentry> <term><option>-B</option> <replaceable>new_bindir</></term> <term><option>--new-bindir=</option><replaceable>new_bindir</></term> <listitem><para>the new cluster executable directory; ! environment variable <envar>PGBINNEW</></para></listitem> </varlistentry> <varlistentry> *************** *** 80,93 **** <term><option>-d</option> <replaceable>old_datadir</></term> <term><option>--old-datadir=</option><replaceable>old_datadir</></term> <listitem><para>the old cluster data directory; environment ! variable <envar>OLDDATADIR</></para></listitem> </varlistentry> <varlistentry> <term><option>-D</option> <replaceable>new_datadir</></term> <term><option>--new-datadir=</option><replaceable>new_datadir</></term> <listitem><para>the new cluster data directory; environment ! variable <envar>NEWDATADIR</></para></listitem> </varlistentry> <varlistentry> --- 80,93 ---- <term><option>-d</option> <replaceable>old_datadir</></term> <term><option>--old-datadir=</option><replaceable>old_datadir</></term> <listitem><para>the old cluster data directory; environment ! variable <envar>PGDATAOLD</></para></listitem> </varlistentry> <varlistentry> <term><option>-D</option> <replaceable>new_datadir</></term> <term><option>--new-datadir=</option><replaceable>new_datadir</></term> <listitem><para>the new cluster data directory; environment ! variable <envar>PGDATANEW</></para></listitem> </varlistentry> <varlistentry> *************** *** 118,131 **** <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 ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> <listitem><para>the new cluster port number; environment ! variable <envar>PGPORT</></para></listitem> </varlistentry> <varlistentry> --- 118,131 ---- <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 ! variable <envar>PGPORTOLD</></para></listitem> </varlistentry> <varlistentry> <term><option>-P</option> <replaceable>new_port_number</></term> <term><option>--new-port=</option><replaceable>new_portnum</></term> <listitem><para>the new cluster port number; environment ! variable <envar>PGPORTNEW</></para></listitem> </varlistentry> <varlistentry> *************** gmake prefix=/usr/local/pgsql.new instal *** 256,263 **** so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. Also make sure ! pg_upgrade is the only program that can connect to the clusters. </para> </step> --- 256,262 ---- so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. </para> </step> *************** NET STOP pgsql-8.3 (<productname>Postgr *** 303,311 **** copying), but you will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the ! same file system. See <literal>pg_upgrade --help</> for a full ! list of options. ! </para> <para> For Windows users, you must be logged into an administrative account, and --- 302,315 ---- copying), but you will not be able to access your old cluster once you start the new cluster after the upgrade. Link mode also requires that the old and new cluster data directories be in the ! same file system. ! </para> ! ! <para> ! <application>pg_upgrade</> defaults to running servers on port ! 50432 to avoid unintended client connections. See <literal>pg_upgrade ! --help</> for a full list of options. ! </para> <para> For Windows users, you must be logged into an administrative account, and
On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? >> >> I'm not sure which patch you are referring to. > > This one which makes 50432 the default port. There appear to be some other changes mixed into this patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Robert Haas wrote: > >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? > >> > >> I'm not sure which patch you are referring to. > > > > This one which makes 50432 the default port. > > There appear to be some other changes mixed into this patch. The additional changes were to have the existing environment variables begin with "PG", as requested. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > Robert Haas wrote: >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: >> >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? >> >> >> >> I'm not sure which patch you are referring to. >> > >> > This one which makes 50432 the default port. >> >> There appear to be some other changes mixed into this patch. > > The additional changes were to have the existing environment variables > begin with "PG", as requested. It's easier to read the patches if you do separate changes in separate patches. Anyway, I'm a bit nervous about this hunk: + if (old_cluster.port == DEF_PGUPORT) + pg_log(PG_FATAL, "When checking a live old server, " + "you must specify the old server's port number.\n"); Is the implication here that I'm now going to need to specify more than 4 command-line options/environment variables for this to work? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Robert Haas wrote: > >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> > Robert Haas wrote: > >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? > >> >> > >> >> I'm not sure which patch you are referring to. > >> > > >> > This one which makes 50432 the default port. > >> > >> There appear to be some other changes mixed into this patch. > > > > The additional changes were to have the existing environment variables > > begin with "PG", as requested. > > It's easier to read the patches if you do separate changes in separate > patches. Anyway, I'm a bit nervous about this hunk: > > + if (old_cluster.port == DEF_PGUPORT) > + pg_log(PG_FATAL, "When checking a live old server, " > + "you must specify the old server's port number.\n"); > > Is the implication here that I'm now going to need to specify more > than 4 command-line options/environment variables for this to work? Yes, we don't inherit PGPORT anymore. Doing anything else was too complex to explain in the docs. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Mon, Jun 27, 2011 at 2:27 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > Robert Haas wrote: >> >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: >> >> > Robert Haas wrote: >> >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: >> >> >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? >> >> >> >> >> >> I'm not sure which patch you are referring to. >> >> > >> >> > This one which makes 50432 the default port. >> >> >> >> There appear to be some other changes mixed into this patch. >> > >> > The additional changes were to have the existing environment variables >> > begin with "PG", as requested. >> >> It's easier to read the patches if you do separate changes in separate >> patches. Anyway, I'm a bit nervous about this hunk: >> >> + if (old_cluster.port == DEF_PGUPORT) >> + pg_log(PG_FATAL, "When checking a live old server, " >> + "you must specify the old server's port number.\n"); >> >> Is the implication here that I'm now going to need to specify more >> than 4 command-line options/environment variables for this to work? > > Yes, we don't inherit PGPORT anymore. Doing anything else was too > complex to explain in the docs. Seems like a usability regression. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Bruce Momjian wrote: > Robert Haas wrote: > > On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce@momjian.us> wrote: > > > Robert Haas wrote: > > >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > > >> > Robert Haas wrote: > > >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: > > >> >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? > > >> >> > > >> >> I'm not sure which patch you are referring to. > > >> > > > >> > This one which makes 50432 the default port. > > >> > > >> There appear to be some other changes mixed into this patch. > > > > > > The additional changes were to have the existing environment variables > > > begin with "PG", as requested. > > > > It's easier to read the patches if you do separate changes in separate > > patches. Anyway, I'm a bit nervous about this hunk: > > > > + if (old_cluster.port == DEF_PGUPORT) > > + pg_log(PG_FATAL, "When checking a live old server, " > > + "you must specify the old server's port number.\n"); > > > > Is the implication here that I'm now going to need to specify more > > than 4 command-line options/environment variables for this to work? > > Yes, we don't inherit PGPORT anymore. Doing anything else was too > complex to explain in the docs. But only if you are running --check on a live server. Otherwise, we will just default to 50432 instead of 5432/PGPORT. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Mon, Jun 27, 2011 at 2:34 PM, Bruce Momjian <bruce@momjian.us> wrote: > Bruce Momjian wrote: >> Robert Haas wrote: >> > On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > > Robert Haas wrote: >> > >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > >> > Robert Haas wrote: >> > >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > >> >> > OK, fair enough. ?Should I apply my ports patch to Postgres 9.2? >> > >> >> >> > >> >> I'm not sure which patch you are referring to. >> > >> > >> > >> > This one which makes 50432 the default port. >> > >> >> > >> There appear to be some other changes mixed into this patch. >> > > >> > > The additional changes were to have the existing environment variables >> > > begin with "PG", as requested. >> > >> > It's easier to read the patches if you do separate changes in separate >> > patches. Anyway, I'm a bit nervous about this hunk: >> > >> > + if (old_cluster.port == DEF_PGUPORT) >> > + pg_log(PG_FATAL, "When checking a live old server, " >> > + "you must specify the old server's port number.\n"); >> > >> > Is the implication here that I'm now going to need to specify more >> > than 4 command-line options/environment variables for this to work? >> >> Yes, we don't inherit PGPORT anymore. Doing anything else was too >> complex to explain in the docs. > > But only if you are running --check on a live server. Otherwise, we > will just default to 50432 instead of 5432/PGPORT. Oh... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On mån, 2011-06-27 at 14:34 -0400, Bruce Momjian wrote: > Bruce Momjian wrote: > > Robert Haas wrote: > > > It's easier to read the patches if you do separate changes in separate > > > patches. Anyway, I'm a bit nervous about this hunk: > > > > > > + if (old_cluster.port == DEF_PGUPORT) > > > + pg_log(PG_FATAL, "When checking a live old server, " > > > + "you must specify the old server's port number.\n"); > > > > > > Is the implication here that I'm now going to need to specify more > > > than 4 command-line options/environment variables for this to work? > > > > Yes, we don't inherit PGPORT anymore. Doing anything else was too > > complex to explain in the docs. > > But only if you are running --check on a live server. Otherwise, we > will just default to 50432 instead of 5432/PGPORT. "When checking a live server, the built-in default port number or the value of the environment variable PGPORT is used. But when performing an upgrade, a different port number is used by default, namely 50432, which can be overridden XXX [how?]" Seems pretty clear to me, as long as that last bit is figured out.
Peter Eisentraut wrote: > On m?n, 2011-06-27 at 14:34 -0400, Bruce Momjian wrote: > > Bruce Momjian wrote: > > > Robert Haas wrote: > > > > It's easier to read the patches if you do separate changes in separate > > > > patches. Anyway, I'm a bit nervous about this hunk: > > > > > > > > + if (old_cluster.port == DEF_PGUPORT) > > > > + pg_log(PG_FATAL, "When checking a live old server, " > > > > + "you must specify the old server's port number.\n"); > > > > > > > > Is the implication here that I'm now going to need to specify more > > > > than 4 command-line options/environment variables for this to work? > > > > > > Yes, we don't inherit PGPORT anymore. Doing anything else was too > > > complex to explain in the docs. > > > > But only if you are running --check on a live server. Otherwise, we > > will just default to 50432 instead of 5432/PGPORT. > > "When checking a live server, the built-in default port number or the > value of the environment variable PGPORT is used. But when performing > an upgrade, a different port number is used by default, namely 50432, > which can be overridden XXX [how?]" > > Seems pretty clear to me, as long as that last bit is figured out. Not sure where you got that text --- I assume that was an old email. I decided it was too complex to have PGPORT be honoroed only if there is a live server, so I just always default to 50432 for both servers, and I added this error check: if (user_opts.check && is_server_running(old_cluster.pgdata)) { *live_check = true; + if (old_cluster.port == DEF_PGUPORT) + pg_log(PG_FATAL, "When checking a live old server, " + "you must specify the old server's port number.\n"); OK? -- 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 m?n, 2011-06-27 at 14:34 -0400, Bruce Momjian wrote: > > > Bruce Momjian wrote: > > > > Robert Haas wrote: > > > > > It's easier to read the patches if you do separate changes in separate > > > > > patches. Anyway, I'm a bit nervous about this hunk: > > > > > > > > > > + if (old_cluster.port == DEF_PGUPORT) > > > > > + pg_log(PG_FATAL, "When checking a live old server, " > > > > > + "you must specify the old server's port number.\n"); > > > > > > > > > > Is the implication here that I'm now going to need to specify more > > > > > than 4 command-line options/environment variables for this to work? > > > > > > > > Yes, we don't inherit PGPORT anymore. Doing anything else was too > > > > complex to explain in the docs. > > > > > > But only if you are running --check on a live server. Otherwise, we > > > will just default to 50432 instead of 5432/PGPORT. > > > > "When checking a live server, the built-in default port number or the > > value of the environment variable PGPORT is used. But when performing > > an upgrade, a different port number is used by default, namely 50432, > > which can be overridden XXX [how?]" > > > > Seems pretty clear to me, as long as that last bit is figured out. > > Not sure where you got that text --- I assume that was an old email. I > decided it was too complex to have PGPORT be honoroed only if there is a > live server, so I just always default to 50432 for both servers, and I > added this error check: > > if (user_opts.check && is_server_running(old_cluster.pgdata)) > { > *live_check = true; > + if (old_cluster.port == DEF_PGUPORT) > + pg_log(PG_FATAL, "When checking a live old server, " > + "you must specify the old server's port number.\n"); > > OK? OK, seeing no replies, I have applied this patch to 9.2, and added documentation to 9.0 and 9.1 suggesting using a non-default port number to avoid unintended client connections --- 9.0/9.1 doc patch attached. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index b24c1e7..e4f90d9 *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *************** gmake prefix=/usr/local/pgsql.new instal *** 256,263 **** so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. Also make sure ! pg_upgrade is the only program that can connect to the clusters. </para> </step> --- 256,262 ---- so you might want to set authentication to <literal>trust</> in <filename>pg_hba.conf</>, or if using <literal>md5</> authentication, use a <filename>~/.pgpass</> file (see <xref linkend="libpq-pgpass">) ! to avoid being prompted repeatedly for a password. </para> </step> *************** pg_upgrade.exe *** 336,341 **** --- 335,343 ---- <para> Obviously, no one should be accessing the clusters during the upgrade. + Consider using a non-default port number, e.g. 50432, for old + and new clusters to avoid unintended client connections during + the upgrade. </para> <para>