Re: 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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> wrote:
> Bruce Momjian wrote:
>> Tom Lane wrote:
>> > Bruce Momjian <bruce(at)momjian(dot)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(at)dunslane(dot)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(at)dunslane(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)gmail(dot)com> writes:
> On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)gmail(dot)com> writes:
> > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)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(at)gmail(dot)com> writes:
> > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce,

* Bruce Momjian (bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

* Bruce Momjian (bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Wed, Jun 15, 2011 at 09:14:16PM -0400, Stephen Frost wrote:
> Bruce,
>
> * Bruce Momjian (bruce(at)momjian(dot)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(at)rice(dot)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(at)rice(dot)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(at)momjian(dot)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(at)rice(dot)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(at)momjian(dot)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(at)rice(dot)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(at)rice(dot)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(at)momjian(dot)us> wrote:
> Tom Lane wrote:
>> "Ross J. Reedstrom" <reedstrm(at)rice(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> wrote:
> Robert Haas wrote:
>> On Wed, Jun 15, 2011 at 1:35 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)gmx(dot)net> writes:
> On ons, 2011-06-15 at 17:50 -0400, Tom Lane wrote:
>> Bruce Momjian <bruce(at)momjian(dot)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(at)gmx(dot)net> writes:
> > On ons, 2011-06-15 at 17:50 -0400, Tom Lane wrote:
> >> Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Tom Lane wrote:
> Christopher Browne <cbbrowne(at)gmail(dot)com> writes:
> > On Wed, Jun 15, 2011 at 5:35 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

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(at)gmx(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

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/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?

--
Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)commandprompt(dot)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(at)commandprompt(dot)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(at)momjian(dot)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(at)commandprompt(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> wrote:
> Robert Haas wrote:
>> On Sun, Jun 26, 2011 at 10:33 PM, Alvaro Herrera
>> <alvherre(at)commandprompt(dot)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(at)momjian(dot)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(at)momjian(dot)us> wrote:
> > Robert Haas wrote:
> >> On Sun, Jun 26, 2011 at 10:33 PM, Alvaro Herrera
> >> <alvherre(at)commandprompt(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Robert Haas wrote:
>> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> wrote:
> > Robert Haas wrote:
> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> wrote:
> Robert Haas wrote:
>> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> > Robert Haas wrote:
>> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> wrote:
> > Robert Haas wrote:
> >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >> > Robert Haas wrote:
> >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian wrote:
> Robert Haas wrote:
> > On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > > Robert Haas wrote:
> > >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > >> > Robert Haas wrote:
> > >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> wrote:
> Robert Haas wrote:
>> On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> > Robert Haas wrote:
>> >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> >> > Robert Haas wrote:
>> >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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

On Mon, Jun 27, 2011 at 2:34 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Bruce Momjian wrote:
>> Robert Haas wrote:
>> > On Mon, Jun 27, 2011 at 2:19 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> > > Robert Haas wrote:
>> > >> On Mon, Jun 27, 2011 at 1:59 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> > >> > Robert Haas wrote:
>> > >> >> On Mon, Jun 27, 2011 at 1:49 PM, Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +