pgsql: pg_upgrade: Remove PGPORT handling from test suite

Peter Eisentraut <peter_e@gmx.net> writes:

pg_upgrade: Remove PGPORT handling from test suite

This patch appears to have broken the ability to run "make check"
for pg_upgrade when there's an installed server running at the build's
default PGPORT. I get a bleat about how the port is already in use...

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On Fri, 2013-05-03 at 21:23 -0400, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

pg_upgrade: Remove PGPORT handling from test suite

This patch appears to have broken the ability to run "make check"
for pg_upgrade when there's an installed server running at the build's
default PGPORT. I get a bleat about how the port is already in use...

Still works for me:

make check PGPORT=55555

What were you using?

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Peter Eisentraut <peter_e@gmx.net> writes:

On Fri, 2013-05-03 at 21:23 -0400, Tom Lane wrote:

This patch appears to have broken the ability to run "make check"
for pg_upgrade when there's an installed server running at the build's
default PGPORT. I get a bleat about how the port is already in use...

Still works for me:
make check PGPORT=55555

What were you using?

Just "make check". Why should I have to do something else?

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

I wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On Fri, 2013-05-03 at 21:23 -0400, Tom Lane wrote:

This patch appears to have broken the ability to run "make check"
for pg_upgrade when there's an installed server running at the build's
default PGPORT. I get a bleat about how the port is already in use...

Still works for me:
make check PGPORT=55555
What were you using?

Just "make check". Why should I have to do something else?

To press the point a little bit: IMO it's a bad idea for pg_upgrade,
alone among contrib modules, to require manual management of the port
choice during "make check". In every other contrib module, pg_regress.c
will go to quite substantial lengths to pick an unused port number for
the temporary installation. I object to having contrib/pg_upgrade
break that for what appears to be no reason at all. I also note that
the pg_upgrade test script, like pg_regress, intentionally unsets every
other libpq connection-control environment variable; so it's quite
unclear why it should honor an external setting for this one.

It's probably not really necessary for the test script to try to
duplicate the dynamic port-number testing done in pg_regress.c
(especially since that isn't terribly bulletproof anyway). However,
I think it should at least replicate this bit of logic:

/*
* To reduce chances of interference with parallel installations, use
* a port number starting in the private range (49152-65535)
* calculated from the version number.
*/
port = 0xC000 | (PG_VERSION_NUM & 0x3FFF);

since that should only take a couple of lines of shell scripting,
and is enough to avoid collisions in ordinary cases.

Barring objection, I'm going to revert
3d53173e20d151341f894f79d556768c845ba3e4 and do that instead.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On Fri, 2013-05-10 at 18:17 -0400, Tom Lane wrote:

It's probably not really necessary for the test script to try to
duplicate the dynamic port-number testing done in pg_regress.c
(especially since that isn't terribly bulletproof anyway). However,
I think it should at least replicate this bit of logic:

/*
* To reduce chances of interference with parallel
installations, use
* a port number starting in the private range (49152-65535)
* calculated from the version number.
*/
port = 0xC000 | (PG_VERSION_NUM & 0x3FFF);

since that should only take a couple of lines of shell scripting,
and is enough to avoid collisions in ordinary cases.

That would be a good idea, but it seems independent of the code I
removed.

The only reason that the pg_upgrade test suite stayed out of the way of
the default port number is that it used a different hard-coded port
number, which also happened to be the production pg_upgrade port number.
That code was put in there before pg_upgrade itself switched to using
50432 by default.

The effect of having left that code in there was that multiple
concurrent pg_upgrade tests in different code trees with different
default ports would interfere with each other. That problem still
exists on platforms without Unix-domain sockets, however.

I think the complete solution here is something like

random_port=$(secret algorithm)
PGPORT=$random_port
PGOLDPORT=$random_port
PGNEWPORT=$random_port
export PGPORT PGOLDPORT PGNEWPORT

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Peter Eisentraut <peter_e@gmx.net> writes:

I think the complete solution here is something like

random_port=$(secret algorithm)
PGPORT=$random_port
PGOLDPORT=$random_port
PGNEWPORT=$random_port
export PGPORT PGOLDPORT PGNEWPORT

Sounds reasonable, will do.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers