Thread: pgsql: pg_upgrade: Remove PGPORT handling from test suite
pg_upgrade: Remove PGPORT handling from test suite This code was left over from when pg_upgrade paid attention to PGPORT. Now it would only affects the regression test run before the test run of pg_upgrade. You can still set PGPORT for that, but there is no reason to have the test driver default it to 50432. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/3d53173e20d151341f894f79d556768c845ba3e4 Modified Files -------------- contrib/pg_upgrade/test.sh | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
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
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?
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
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
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
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