Обсуждение: Regression tests versus the buildfarm environment

There's an interesting buildfarm failure here:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=polecat&dt=2010-08-10%2023:46:10
It appears to me that this was caused by the concurrent run of another
buildfarm animal on the same physical machine, namely:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=colugos&dt=2010-08-11%2000:02:58
Both animals are trying to test HEAD, which means that pg_regress
defaults to the same postmaster port number in both builds:
   if (temp_install && !port_specified_by_user)
       /*        * To reduce chances of interference with parallel installations, use        * a port number starting
inthe private range (49152-65535)        * calculated from the version number.        */       port = 0xC000 |
(PG_VERSION_NUM& 0x3FFF);
 

We observe colugos successfully starting on that port:

============== starting postmaster                    ==============
running on port 57332 with pid 47019
============== creating database "regression"         ==============
CREATE DATABASE
ALTER DATABASE
... etc etc ...

polecat comes along what must be only moments later, and tries to use
the same port for its temp install:

============== starting postmaster                    ==============
running on port 57332 with pid 47022
============== creating database "regression"         ==============
ERROR:  duplicate key value violates unique constraint "pg_database_datname_index"
DETAIL:  Key (datname)=(regression) already exists.
command failed:
"/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/install//usr/local/src/build-farm-3.2/builds/HEAD/inst/bin/psql"
-X-c "CREATE DATABASE \"regression\" TEMPLATE=template0 ENCODING='SQL_ASCII' LC_COLLATE='C' LC_CTYPE='C'" "postgres"
 
pg_ctl: PID file
"/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/data/postmaster.pid"does not exist
 
Is server running?

pg_regress: could not stop postmaster: exit code was 256

Now the postmaster log shows that the second postmaster correctly
recognized that the port number was already in use, so it bailed out:

================== pgsql.15278/src/test/regress/log/postmaster.log ===================
[4c61f2d2.b7ae:1] FATAL:  lock file "/tmp/.s.PGSQL.57332.lock" already exists
[4c61f2d2.b7ae:2] HINT:  Is another postmaster (PID 47019) using socket file "/tmp/.s.PGSQL.57332"?

However, pg_regress failed to have a clue about what had happened,
and bulled ahead trying to run the regression tests (against the
postmaster started by the other pg_regress instance).  A look at the
code shows that it is merely trying to run psql, and if psql reports
that it can connect to the specified port, then pg_regress thinks the
postmaster started OK.  Of course, psql was really reporting that it
could connect to the other instance's postmaster.


I've seen similar multiple-postmaster-interference symptoms before in
the buildfarm, but never really understood the cause.

I am not sure if there's anything very good we can do about the
problem of pg_regress misidentifying the postmaster it's managed to
connect to.  A real solution would probably be much more trouble than
it's worth, anyway.  However, it does seem like we ought to be able to
do something about two buildfarm critters defaulting to the same choice
of port number.  The buildfarm infrastructure goes to great lengths to
pick nonconflicting port numbers for the "installed" postmasters it
runs; but we're ignoring all that effort and just using a hardwired
port number for "make check".  This is dumb.

pg_regress does have a --port argument that can be used to override
that default.  I don't know whether the buildfarm script calls
pg_regress directly or does "make check".  If the latter, we'd need to
twiddle the Makefiles to allow a port number to get passed in.  But
this seems well worthwhile to me.

Comments?
        regards, tom lane

<div class="gmail_quote">On Wed, Aug 11, 2010 at 06:42, Tom Lane <span dir="ltr"><<a
href="mailto:tgl@sss.pgh.pa.us">tgl@sss.pgh.pa.us</a>></span>wrote:<br /><blockquote class="gmail_quote"
style="margin:0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div id=":1h2">I am not
sureif there's anything very good we can do about the<br /> problem of pg_regress misidentifying the postmaster it's
managedto<br /> connect to.  A real solution would probably be much more trouble than<br /> it's worth, anyway.
 However,it does seem like we ought to be able to<br /> do something about two buildfarm critters defaulting to the
samechoice<br /> of port number.  The buildfarm infrastructure goes to great lengths to<br /> pick nonconflicting port
numbersfor the "installed" postmasters it<br /> runs; but we're ignoring all that effort and just using a hardwired<br
/>port number for "make check".  This is dumb.<br /><br /> pg_regress does have a --port argument that can be used to
override<br/> that default.  I don't know whether the buildfarm script calls<br /> pg_regress directly or does "make
check". If the latter, we'd need to<br /> twiddle the Makefiles to allow a port number to get passed in.  But<br />
thisseems well worthwhile to me.<br /><br /> Comments?</div></blockquote></div><br />We just put in the possibility to
namethe client connections.  Would it be interesting to be able to name the server installation itself?<br /> 

On 08/11/2010 12:42 AM, Tom Lane wrote:
> There's an interesting buildfarm failure here:
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=polecat&dt=2010-08-10%2023:46:10
> It appears to me that this was caused by the concurrent run of another
> buildfarm animal on the same physical machine, namely:
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=colugos&dt=2010-08-11%2000:02:58
> Both animals are trying to test HEAD, which means that pg_regress
> defaults to the same postmaster port number in both builds:
>
>      if (temp_install&&  !port_specified_by_user)
>
>          /*
>           * 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);
>
> We observe colugos successfully starting on that port:
>
> ============== starting postmaster                    ==============
> running on port 57332 with pid 47019
> ============== creating database "regression"         ==============
> CREATE DATABASE
> ALTER DATABASE
> ... etc etc ...
>
> polecat comes along what must be only moments later, and tries to use
> the same port for its temp install:
>
> ============== starting postmaster                    ==============
> running on port 57332 with pid 47022
> ============== creating database "regression"         ==============
> ERROR:  duplicate key value violates unique constraint "pg_database_datname_index"
> DETAIL:  Key (datname)=(regression) already exists.
> command failed:
"/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/install//usr/local/src/build-farm-3.2/builds/HEAD/inst/bin/psql"
-X-c "CREATE DATABASE \"regression\" TEMPLATE=template0 ENCODING='SQL_ASCII' LC_COLLATE='C' LC_CTYPE='C'" "postgres"
 
> pg_ctl: PID file
"/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/data/postmaster.pid"does not exist
 
> Is server running?
>
> pg_regress: could not stop postmaster: exit code was 256
>
> Now the postmaster log shows that the second postmaster correctly
> recognized that the port number was already in use, so it bailed out:
>
> ================== pgsql.15278/src/test/regress/log/postmaster.log ===================
> [4c61f2d2.b7ae:1] FATAL:  lock file "/tmp/.s.PGSQL.57332.lock" already exists
> [4c61f2d2.b7ae:2] HINT:  Is another postmaster (PID 47019) using socket file "/tmp/.s.PGSQL.57332"?
>
> However, pg_regress failed to have a clue about what had happened,
> and bulled ahead trying to run the regression tests (against the
> postmaster started by the other pg_regress instance).  A look at the
> code shows that it is merely trying to run psql, and if psql reports
> that it can connect to the specified port, then pg_regress thinks the
> postmaster started OK.  Of course, psql was really reporting that it
> could connect to the other instance's postmaster.
>
>
> I've seen similar multiple-postmaster-interference symptoms before in
> the buildfarm, but never really understood the cause.
>
> I am not sure if there's anything very good we can do about the
> problem of pg_regress misidentifying the postmaster it's managed to
> connect to.  A real solution would probably be much more trouble than
> it's worth, anyway.  However, it does seem like we ought to be able to
> do something about two buildfarm critters defaulting to the same choice
> of port number.  The buildfarm infrastructure goes to great lengths to
> pick nonconflicting port numbers for the "installed" postmasters it
> runs; but we're ignoring all that effort and just using a hardwired
> port number for "make check".  This is dumb.
>
> pg_regress does have a --port argument that can be used to override
> that default.  I don't know whether the buildfarm script calls
> pg_regress directly or does "make check".  If the latter, we'd need to
> twiddle the Makefiles to allow a port number to get passed in.  But
> this seems well worthwhile to me.
>
> Comments?
>
>             

The buildfarm calls "make check".

Why not just add the configured port (DEF_PGPORT) into the calculation 
of the port to run on?

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> On 08/11/2010 12:42 AM, Tom Lane wrote:
>> ...  However, it does seem like we ought to be able to
>> do something about two buildfarm critters defaulting to the same choice
>> of port number.

> Why not just add the configured port (DEF_PGPORT) into the calculation 
> of the port to run on?

No, that would be just about the worst possible choice.  It'd be
guaranteed to fail in the standard scenario that you are running
"make check" before updating an existing installation.

I think what we want to do here is arrange for the buildfarm script to
select the same port that it's going to use later for an "installed"
postmaster, but it has to go via a different path than DEF_PGPORT.

The first thought that comes to mind is to adjust the makefiles
like this:
ifdef REGRESSION_TEST_PORT... add --port $(REGRESSION_TEST_PORT) to pg_regress flags ...endif

and then the buildfarm script could use
make REGRESSION_TEST_PORT=nnn check

But I'm not sure what the cleanest way is if we have to pass that
down from the top-level makefile.  Make doesn't pass down variables
automatically does it?

Another possibility is to allow a regression test port number to
be configured via configure; though that seems like a slightly
larger change than I'd want to push into the back branches.
        regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> A look at the code shows that it is merely trying to run psql, and
> if psql reports that it can connect to the specified port, then
> pg_regress thinks the postmaster started OK.  Of course, psql was
> really reporting that it could connect to the other instance's
> postmaster.
Clearly picking unique ports for `make check` is the ultimate
solution, but I'm curious whether this would have been caught sooner
with less effort if the pg_ctl TODO titled "Have the postmaster
write a random number to a file on startup that pg_ctl checks
against the contents of a pg_ping response on its initial connection
(without login)" had been implemented.
http://archives.postgresql.org/pgsql-bugs/2009-10/msg00110.php
It sounds like it's related; but was curious to confirm.
-Kevin

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> A look at the code shows that it is merely trying to run psql, and
>> if psql reports that it can connect to the specified port, then
>> pg_regress thinks the postmaster started OK.  Of course, psql was
>> really reporting that it could connect to the other instance's
>> postmaster.
> Clearly picking unique ports for `make check` is the ultimate
> solution, but I'm curious whether this would have been caught sooner
> with less effort if the pg_ctl TODO titled "Have the postmaster
> write a random number to a file on startup that pg_ctl checks
> against the contents of a pg_ping response on its initial connection
> (without login)" had been implemented.

It would certainly make the failure more transparent.  As I mentioned,
there are previous buildfarm failures that look like they might be
caused by a similar conflict, but it's seldom possible to be sure.
A cross-check like that would be much safer.

BTW, I don't know why anyone would think that "a random number" would
offer any advantage here.  I'd use the postmaster PID, which is
guaranteed to be unique across the space that you're worried about.
In fact, you could implement this off the existing postmaster.pid,
no need for any new file.  What's lacking is the pg_ping protocol.
        regards, tom lane

On 08/11/2010 09:43 AM, Tom Lane wrote:
> Andrew Dunstan<andrew@dunslane.net>  writes:
>> On 08/11/2010 12:42 AM, Tom Lane wrote:
>>> ...  However, it does seem like we ought to be able to
>>> do something about two buildfarm critters defaulting to the same choice
>>> of port number.
>> Why not just add the configured port (DEF_PGPORT) into the calculation
>> of the port to run on?
> No, that would be just about the worst possible choice.  It'd be
> guaranteed to fail in the standard scenario that you are running
> "make check" before updating an existing installation.

One of us is missing something. I didn't say to run the checks using the 
configured port. I had in mind something like:
    port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) &  0x3FFF);


cheers

andrew

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> BTW, I don't know why anyone would think that "a random number"
> would offer any advantage here.  I'd use the postmaster PID, which
> is guaranteed to be unique across the space that you're worried
> about.
Well, in the post I cited, it was you who argued that the PID was a
bad choice, suggested a random number, and stated "That would have a
substantially lower collision probability than PID, if the number
generation process were well designed; and it wouldn't risk exposing
anything sensitive in the ping response."
-Kevin

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> BTW, I don't know why anyone would think that "a random number"
>> would offer any advantage here.  I'd use the postmaster PID, which
>> is guaranteed to be unique across the space that you're worried
>> about.
> Well, in the post I cited, it was you who argued that the PID was a
> bad choice, suggested a random number, and stated "That would have a
> substantially lower collision probability than PID, if the number
> generation process were well designed; and it wouldn't risk exposing
> anything sensitive in the ping response."

Hmm. I don't remember why we'd think that the postmaster PID was
sensitive information ... but if you take that as true, then yeah
it couldn't be included in a pg_ping response.
        regards, tom lane

Andrew Dunstan <andrew@dunslane.net> writes:
> On 08/11/2010 09:43 AM, Tom Lane wrote:
>> Andrew Dunstan<andrew@dunslane.net>  writes:
>>> Why not just add the configured port (DEF_PGPORT) into the calculation
>>> of the port to run on?

>> No, that would be just about the worst possible choice.  It'd be
>> guaranteed to fail in the standard scenario that you are running
>> "make check" before updating an existing installation.

> One of us is missing something. I didn't say to run the checks using the 
> configured port. I had in mind something like:

>      port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) &  0x3FFF);

Oh, I see, modify the DEF_PGPORT don't just use it as-is.  OK, except
that I think something like the above is still pretty risky for the
buildfarm, because you would still have conflicts for assorted
combinations of version numbers and branch_port settings.

How about just this:
    port = 0xC000 | (DEF_PGPORT & 0x3FFF);

If anyone was actually using a DEF_PGPORT above 0xC000, this would mean
that they couldn't run "make check" on the same machine as their running
installation (at least not without adjusting pg_regress's port choice,
which I still think we need to tweak the makefiles to make easier).
But for ordinary buildfarm usage, this would be guaranteed not to
conflict as long as you'd chosen nonconflicting branch_ports for all
your branches and animals.

Or we could do something like
    port = 0xC000 ^ (DEF_PGPORT & 0x7FFF);

which is absolutely guaranteed not to conflict with DEF_PGPORT, at the
cost of possibly shifting into the 32K-48K port number range if you
had set DEF_PGPORT above 48K.
        regards, tom lane

On Wed, Aug 11, 2010 at 10:15 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 08/11/2010 09:43 AM, Tom Lane wrote:
>>> Andrew Dunstan<andrew@dunslane.net>  writes:
>>>> Why not just add the configured port (DEF_PGPORT) into the calculation
>>>> of the port to run on?
>
>>> No, that would be just about the worst possible choice.  It'd be
>>> guaranteed to fail in the standard scenario that you are running
>>> "make check" before updating an existing installation.
>
>> One of us is missing something. I didn't say to run the checks using the
>> configured port. I had in mind something like:
>
>>      port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) &  0x3FFF);
>
> Oh, I see, modify the DEF_PGPORT don't just use it as-is.  OK, except
> that I think something like the above is still pretty risky for the
> buildfarm, because you would still have conflicts for assorted
> combinations of version numbers and branch_port settings.
>
> How about just this:
>
>     port = 0xC000 | (DEF_PGPORT & 0x3FFF);
>
> If anyone was actually using a DEF_PGPORT above 0xC000, this would mean
> that they couldn't run "make check" on the same machine as their running
> installation (at least not without adjusting pg_regress's port choice,
> which I still think we need to tweak the makefiles to make easier).
> But for ordinary buildfarm usage, this would be guaranteed not to
> conflict as long as you'd chosen nonconflicting branch_ports for all
> your branches and animals.
>
> Or we could do something like
>
>     port = 0xC000 ^ (DEF_PGPORT & 0x7FFF);
>
> which is absolutely guaranteed not to conflict with DEF_PGPORT, at the
> cost of possibly shifting into the 32K-48K port number range if you
> had set DEF_PGPORT above 48K.

I like XOR a lot better than OR.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Vik Reykja <vikreykja@gmail.com> writes:
> We just put in the possibility to name the client connections.  Would it be
> interesting to be able to name the server installation itself?

Wouldn't do anything for this problem --- it would just introduce
something else the buildfarm would have to worry about uniqueness of.
        regards, tom lane

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 11, 2010 at 10:15 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Or we could do something like
>> 
>> � � port = 0xC000 ^ (DEF_PGPORT & 0x7FFF);
>> 
>> which is absolutely guaranteed not to conflict with DEF_PGPORT, at the
>> cost of possibly shifting into the 32K-48K port number range if you
>> had set DEF_PGPORT above 48K.

> I like XOR a lot better than OR.

Yeah, on reflection that seems better.  Barring objection I will see
about making this happen in all live branches.
        regards, tom lane

On 08/11/2010 10:23 AM, Robert Haas wrote:
>
>> Or we could do something like
>>
>>      port = 0xC000 ^ (DEF_PGPORT&  0x7FFF);
>>
>> which is absolutely guaranteed not to conflict with DEF_PGPORT, at the
>> cost of possibly shifting into the 32K-48K port number range if you
>> had set DEF_PGPORT above 48K.
> I like XOR a lot better than OR.
>

For years we told people to make sure they picked 4 digit port numbers 
for the buildfarm, and while I removed that note recently it can be put 
back. So I don't think there's much danger - let's got with XOR.


cheers

andrew

On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote:
> > One of us is missing something. I didn't say to run the checks using
> the 
> > configured port. I had in mind something like:
> 
> >      port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) &  0x3FFF);
> 
> Oh, I see, modify the DEF_PGPORT don't just use it as-is.  OK, except
> that I think something like the above is still pretty risky for the
> buildfarm, because you would still have conflicts for assorted
> combinations of version numbers and branch_port settings.
> 
> How about just this:
> 
>      port = 0xC000 | (DEF_PGPORT & 0x3FFF);

The version number was put in there intentionally, for developers who
work on multiple branches at once.  That's the whole reason this code
exists.  Please don't remove it.

On ons, 2010-08-11 at 09:55 -0400, Tom Lane wrote:
> BTW, I don't know why anyone would think that "a random number" would
> offer any advantage here.  I'd use the postmaster PID, which is
> guaranteed to be unique across the space that you're worried about.
> In fact, you could implement this off the existing postmaster.pid,
> no need for any new file.  What's lacking is the pg_ping protocol.

Why not just compare pg_backend_pid() with postmaster.pid?

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2010-08-11 at 09:55 -0400, Tom Lane wrote:
>> BTW, I don't know why anyone would think that "a random number" would
>> offer any advantage here.  I'd use the postmaster PID, which is
>> guaranteed to be unique across the space that you're worried about.
>> In fact, you could implement this off the existing postmaster.pid,
>> no need for any new file.  What's lacking is the pg_ping protocol.

> Why not just compare pg_backend_pid() with postmaster.pid?

How's that help?  pg_backend_pid isn't going to return the postmaster's
PID ... maybe we could add a new function that does return the
postmaster's PID, though.
        regards, tom lane

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote:
>> How about just this:
>>    port = 0xC000 | (DEF_PGPORT & 0x3FFF);

> The version number was put in there intentionally, for developers who
> work on multiple branches at once.  That's the whole reason this code
> exists.  Please don't remove it.

I work on multiple branches all day every day.  This wouldn't hinder
me in the slightest, because I use a different DEF_PGPORT for each
branch.  If you don't, it's hard to see how you manage to deal with
multiple branches on one machine ... do you not ever actually install
them?  Even if you don't, changing this would only mean that you
couldn't safely run "make check" concurrently in multiple branches.
        regards, tom lane

Peter Eisentraut <peter_e@gmx.net> wrote:
> On ons, 2010-08-11 at 09:55 -0400, Tom Lane wrote:
>> BTW, I don't know why anyone would think that "a random number"
>> would offer any advantage here.  I'd use the postmaster PID,
>> which is guaranteed to be unique across the space that you're
>> worried about.  In fact, you could implement this off the
>> existing postmaster.pid, no need for any new file.  What's
>> lacking is the pg_ping protocol.
> 
> Why not just compare pg_backend_pid() with postmaster.pid?
See the prior discussion in the archives.  We started with that and
found problems, to which Tom suggested a random number as the best
solution.  Let's at least start any further discussion informed by
what's gone before; if there was a flaw in the reasoning, please
point that out.
-Kevin

On ons, 2010-08-11 at 11:48 -0400, Tom Lane wrote:
> How's that help?  pg_backend_pid isn't going to return the
> postmaster's
> PID ... maybe we could add a new function that does return the
> postmaster's PID, though.

Hmm, is there a portable way to find the parent PID of some other
process, given its PID?

On 08/11/2010 11:42 AM, Peter Eisentraut wrote:
> On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote:
>>> One of us is missing something. I didn't say to run the checks using
>> the
>>> configured port. I had in mind something like:
>>>       port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT)&   0x3FFF);
>> Oh, I see, modify the DEF_PGPORT don't just use it as-is.  OK, except
>> that I think something like the above is still pretty risky for the
>> buildfarm, because you would still have conflicts for assorted
>> combinations of version numbers and branch_port settings.
>>
>> How about just this:
>>
>>       port = 0xC000 | (DEF_PGPORT&  0x3FFF);
> The version number was put in there intentionally, for developers who
> work on multiple branches at once.  That's the whole reason this code
> exists.  Please don't remove it.
>

Do they run "make check" by hand simultaneously on multiple branches? 
That's the only way you'd get a collision here, I think.

cheers

andrew

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Peter Eisentraut <peter_e@gmx.net> wrote:
>> Why not just compare pg_backend_pid() with postmaster.pid?
> See the prior discussion in the archives.  We started with that and
> found problems, to which Tom suggested a random number as the best
> solution.

I think Peter's idea is a bit different though.  The previous concern
was about what information would be okay to expose in a pg_ping response
packet, which presumably would be available to anybody who could open a
connection to the postmaster port.  What he's suggesting is to
crosscheck against data that is available after a successful login.
That eliminates the security complaint.

It strikes me we could do something without adding a postmaster-PID
SQL function, too.  What about doing SHOW DATA_DIRECTORY and comparing
that to what pg_regress expects?
        regards, tom lane

On ons, 2010-08-11 at 11:53 -0400, Andrew Dunstan wrote:
> > The version number was put in there intentionally, for developers
> who
> > work on multiple branches at once.  That's the whole reason this
> code
> > exists.  Please don't remove it.
> >
> 
> Do they run "make check" by hand simultaneously on multiple branches? 
> That's the only way you'd get a collision here, I think.

That is exactly what I'm talking about.

On ons, 2010-08-11 at 11:47 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote:
> >> How about just this:
> >>    port = 0xC000 | (DEF_PGPORT & 0x3FFF);
> 
> > The version number was put in there intentionally, for developers who
> > work on multiple branches at once.  That's the whole reason this code
> > exists.  Please don't remove it.
> 
> I work on multiple branches all day every day.  This wouldn't hinder
> me in the slightest, because I use a different DEF_PGPORT for each
> branch.  If you don't, it's hard to see how you manage to deal with
> multiple branches on one machine ... do you not ever actually install
> them?

No, not nearly as much as I run "make check".

> Even if you don't, changing this would only mean that you
> couldn't safely run "make check" concurrently in multiple branches.

That's exactly the point.  The original discussion is here:
http://archives.postgresql.org/message-id/491D9935.9010200@gmx.net

<br /><br /> On 08/11/2010 02:12 PM, Peter Eisentraut wrote: <blockquote
cite="mid:1281550347.26522.13.camel@vanquo.pezone.net"type="cite"><br /><blockquote type="cite"><pre wrap="">Even if
youdon't, changing this would only mean that you
 
couldn't safely run "make check" concurrently in multiple branches.
</pre></blockquote><pre wrap="">
That's exactly the point.  The original discussion is here:
<a class="moz-txt-link-freetext"
href="http://archives.postgresql.org/message-id/491D9935.9010200@gmx.net">http://archives.postgresql.org/message-id/491D9935.9010200@gmx.net</a>

</pre></blockquote><br /><br /> You original email said:<br /><br /><blockquote><tt>For some historic reasons, I have
mylocal scripts set up so that they </tt><tt>build development instances using the hardcoded port 65432.<br
/></tt></blockquote><br/> I think my response would be "Don't do that".<br /><br /> Having said that, maybe we could
reasonablyuse something like DEF_PGPORT + 10 * major_version + minor_version in the calculation and advise buildfarm
memberswith multiple animals to keep their port ranges say, 200 or more apart.<br /><br /> But maybe we should just
stickwith my earlier advice :-)<br /><br /> cheers<br /><br /> andrew<br /><br /><br /> 

Andrew Dunstan <andrew@dunslane.net> writes:
> You original email said:
>     For some historic reasons, I have my local scripts set up so that
>     they build development instances using the hardcoded port 65432.

> I think my response would be "Don't do that".

Yeah ... or at least use a different port per branch.  Or make use of
the ability to force pg_regress to use a nondefault port (which I still
say we need to make accessible through "make check", whether or not the
buildfarm does it that way).

> Having said that, maybe we could reasonably use something like 
> DEF_PGPORT + 10 * major_version + minor_version in the calculation and 
> advise buildfarm members with multiple animals to keep their port ranges 
> say, 200 or more apart.

I think that just makes it more prone to failure.  We should have the
buildfarm configuration such that any one run uses the same port number
for both installed and uninstalled regression tests.  If Peter is dead
set on not changing pg_regress's default, then changing the makefiles to
enable use of the --port switch is the way to do that.
        regards, tom lane

On 08/11/2010 04:17 PM, Tom Lane wrote:
> We should have the
> buildfarm configuration such that any one run uses the same port number
> for both installed and uninstalled regression tests.  If Peter is dead
> set on not changing pg_regress's default, then changing the makefiles to
> enable use of the --port switch is the way to do that.
>
>             

If you really want it to use the exact same port, then we'll probably 
need to change the Makefiles regardless of how we eventually decide to 
set the default.

Another way would be to have pg_regress honour an environment var like 
PG_REGRESS_PORT, which the buildfarm script could use.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> Another way would be to have pg_regress honour an environment var like 
> PG_REGRESS_PORT, which the buildfarm script could use.

Yeah, that would work too.  (Is it portable to Windows, though?)

I prefer the change-the-default approach mainly because it wouldn't
require any documentation, whereas it'd be a bit hard to argue that
environment variables etc shouldn't be documented ...
        regards, tom lane

On 08/11/2010 04:47 PM, Tom Lane wrote:
> Andrew Dunstan<andrew@dunslane.net>  writes:
>> Another way would be to have pg_regress honour an environment var like
>> PG_REGRESS_PORT, which the buildfarm script could use.
> Yeah, that would work too.  (Is it portable to Windows, though?)

Should be

> I prefer the change-the-default approach mainly because it wouldn't
> require any documentation, whereas it'd be a bit hard to argue that
> environment variables etc shouldn't be documented ...
>
>             

Yeah. The other advantage is that we would not need to wait until we had 
got everyone to update their versions of the buildfarm code. So I agree 
this is the nicest solution.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> On 08/11/2010 04:47 PM, Tom Lane wrote:
>> I prefer the change-the-default approach mainly because it wouldn't
>> require any documentation,

> Yeah. The other advantage is that we would not need to wait until we had 
> got everyone to update their versions of the buildfarm code.

Um.  That's actually a pretty darn strong point, considering how
slow some buildfarm owners are to update the script :-(
        regards, tom lane

On Wed, Aug 11, 2010 at 5:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 08/11/2010 04:47 PM, Tom Lane wrote:
>>> I prefer the change-the-default approach mainly because it wouldn't
>>> require any documentation,
>
>> Yeah. The other advantage is that we would not need to wait until we had
>> got everyone to update their versions of the buildfarm code.
>
> Um.  That's actually a pretty darn strong point, considering how
> slow some buildfarm owners are to update the script :-(

Well, it's a convenient time to get such changes in, now, because
folks are going to need to update their scripts rather soon as a
consequence of the Git migration :-).
--
http://linuxfinances.info/info/linuxdistributions.html

On ons, 2010-08-11 at 16:17 -0400, Tom Lane wrote:
> We should have the buildfarm configuration such that any one run uses
> the same port number for both installed and uninstalled regression
> tests.

I'm getting lost here what the actual requirements are.  The above is
obviously not going to work as a default for pg_regress, because the
port number for an installed test is determined by the user and is
likely to be in the public range, whereas the uninstalled test should
use something from the private range.

On ons, 2010-08-11 at 15:06 -0400, Andrew Dunstan wrote:
> You original email said:
> 
>         For some historic reasons, I have my local scripts set up so
>         that they build development instances using the hardcoded port
>         65432.
> 
> I think my response would be "Don't do that".

Do you have a concrete suggestion for a different way to handle it?

<br /><br /> On 08/12/2010 08:43 AM, Peter Eisentraut wrote: <blockquote
cite="mid:1281617034.4463.7.camel@fsopti579.F-Secure.com"type="cite"><pre wrap="">On ons, 2010-08-11 at 15:06 -0400,
AndrewDunstan wrote:
 
</pre><blockquote type="cite"><pre wrap="">You original email said:
       For some historic reasons, I have my local scripts set up so       that they build development instances using
thehardcoded port       65432.
 

I think my response would be "Don't do that".
</pre></blockquote><pre wrap="">
Do you have a concrete suggestion for a different way to handle it?


</pre></blockquote><br /> Well, I do all my builds under a common directory, and my setup shell script has stuff like
thisto choose a port:<br /><br /><blockquote>for port in `seq -w 5701 5799` ; do<br />    grep -q --
"--with-pgport=$port"$base/*/config.log || break<br /> done<br /><br /></blockquote> It's worked fairly well for me for
aboutfive years now. No doubt there could be many variations on this theme.<br /><br /> cheers<br /><br /> andrew<br /> 

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2010-08-11 at 16:17 -0400, Tom Lane wrote:
>> We should have the buildfarm configuration such that any one run uses
>> the same port number for both installed and uninstalled regression
>> tests.

> I'm getting lost here what the actual requirements are.  The above is
> obviously not going to work as a default for pg_regress, because the
> port number for an installed test is determined by the user and is
> likely to be in the public range, whereas the uninstalled test should
> use something from the private range.

Certainly, but the buildfarm's "installed" test doesn't try to start on
5432.  It starts on whatever branch_port the buildfarm owner has
specified for that animal and that branch.  It's the owner's
responsibility to make that nonconflicting across the services and
buildfarm critters he has running on a given machine.  What I'm saying
is that *in the buildfarm* we want the "make check" case to also use
this predetermined safe port number.  That has nothing whatever to do
with what is good practice for other cases.
        regards, tom lane

<br /><br /> On 08/12/2010 10:22 AM, Tom Lane wrote: <blockquote cite="mid:13573.1281622963@sss.pgh.pa.us"
type="cite"><prewrap="">Peter Eisentraut <a class="moz-txt-link-rfc2396E"
href="mailto:peter_e@gmx.net"><peter_e@gmx.net></a>writes:
 
</pre><blockquote type="cite"><pre wrap="">On ons, 2010-08-11 at 16:17 -0400, Tom Lane wrote:
</pre><blockquote type="cite"><pre wrap="">We should have the buildfarm configuration such that any one run uses
the same port number for both installed and uninstalled regression
tests.
</pre></blockquote></blockquote><pre wrap="">
</pre><blockquote type="cite"><pre wrap="">I'm getting lost here what the actual requirements are.  The above is
obviously not going to work as a default for pg_regress, because the
port number for an installed test is determined by the user and is
likely to be in the public range, whereas the uninstalled test should
use something from the private range.
</pre></blockquote><pre wrap="">
Certainly, but the buildfarm's "installed" test doesn't try to start on
5432.  It starts on whatever branch_port the buildfarm owner has
specified for that animal and that branch.  It's the owner's
responsibility to make that nonconflicting across the services and
buildfarm critters he has running on a given machine.  What I'm saying
is that *in the buildfarm* we want the "make check" case to also use
this predetermined safe port number.  That has nothing whatever to do
with what is good practice for other cases.
        
</pre></blockquote><br /> Well, I think the steps to do that are:<br /><br /><ul><li>change src/test/GNUmakefile to
providefor a TMP_PORT setting that gets passed to pg_regress<li>change src/tools/msvc/vcregress.pl to
match<li>backpatchthese changes to all live branches<br /><li>change the buildfarm script to use the new
options.</ul><br/> I don't think this should preclude changing the way we calculate the default port for pg_regress,
forthe reasons mentioned upthread.<br /><br /> cheers<br /><br /> andrew<br />