Re: pgsql: pg_upgrade: Remove PGPORT handling from test suite

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: pg_upgrade: Remove PGPORT handling from test suite
Date: 2013-05-10 22:17:40
Message-ID: 21022.1368224260@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

I wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2013-05-11 03:08:01 Re: pgsql: pg_upgrade: Remove PGPORT handling from test suite
Previous Message Tom Lane 2013-05-10 21:16:30 pgsql: Guard against input_rows == 0 in estimate_num_groups().