Re: Brittleness in regression test setup - Mailing list pgsql-hackers

Tom Lane wrote:
> One thing we should do is have pg_regress.c, not the Makefile,
> select the default port to use.  The concatenate-5 behavior is
> just not intelligent enough.

How about something like this, constructing a port number from the
version and a timestamp?  We could also take 2 more bits from the
version and give it to the timestamp, which would make this a bit safer,
I think.


Index: src/test/regress/GNUmakefile
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/GNUmakefile,v
retrieving revision 1.75
diff -u -3 -p -r1.75 GNUmakefile
--- src/test/regress/GNUmakefile    1 Oct 2008 22:38:57 -0000    1.75
+++ src/test/regress/GNUmakefile    25 Nov 2008 15:14:19 -0000
@@ -14,9 +14,6 @@ subdir = src/test/regress
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global

-# port number for temp-installation test postmaster
-TEMP_PORT = 5$(DEF_PGPORT)
-
 # file with extra config for temp build
 TEMP_CONF =
 ifdef TEMP_CONFIG
@@ -144,7 +141,7 @@ tablespace-setup:
 pg_regress_call = ./pg_regress --inputdir=$(srcdir) --dlpath=. --multibyte=$(MULTIBYTE) --load-language=plpgsql
$(NOLOCALE)

 check: all
-    $(pg_regress_call) --temp-install=./tmp_check --top-builddir=$(top_builddir) --temp-port=$(TEMP_PORT)
--schedule=$(srcdir)/parallel_schedule$(MAXCONNOPT) $(TEMP_CONF) 
+    $(pg_regress_call) --temp-install=./tmp_check --top-builddir=$(top_builddir)
--schedule=$(srcdir)/parallel_schedule$(MAXCONNOPT) $(TEMP_CONF) 

 installcheck: all
     $(pg_regress_call) --psqldir=$(PSQLDIR) --schedule=$(srcdir)/serial_schedule
@@ -163,7 +160,7 @@ bigtest: all
     $(pg_regress_call) --psqldir=$(PSQLDIR) --schedule=$(srcdir)/serial_schedule numeric_big

 bigcheck: all
-    $(pg_regress_call) --temp-install=./tmp_check --top-builddir=$(top_builddir) --temp-port=$(TEMP_PORT)
--schedule=$(srcdir)/parallel_schedule$(MAXCONNOPT) numeric_big 
+    $(pg_regress_call) --temp-install=./tmp_check --top-builddir=$(top_builddir)
--schedule=$(srcdir)/parallel_schedule$(MAXCONNOPT) numeric_big 


 ##
Index: src/test/regress/pg_regress.c
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/pg_regress.c,v
retrieving revision 1.50
diff -u -3 -p -r1.50 pg_regress.c
--- src/test/regress/pg_regress.c    20 Nov 2008 15:03:39 -0000    1.50
+++ src/test/regress/pg_regress.c    25 Nov 2008 15:14:20 -0000
@@ -83,10 +83,9 @@ static _stringlist *extra_tests = NULL;
 static char *temp_install = NULL;
 static char *temp_config = NULL;
 static char *top_builddir = NULL;
-static int    temp_port = 65432;
 static bool nolocale = false;
 static char *hostname = NULL;
-static int    port = -1;
+static int    port = 0;
 static char *dlpath = PKGLIBDIR;
 static char *user = NULL;
 static _stringlist *extraroles = NULL;
@@ -733,7 +732,7 @@ initialize_environment(void)
         else
             unsetenv("PGHOST");
         unsetenv("PGHOSTADDR");
-        if (port != -1)
+        if (port)
         {
             char        s[16];

@@ -789,7 +788,7 @@ initialize_environment(void)
             doputenv("PGHOST", hostname);
             unsetenv("PGHOSTADDR");
         }
-        if (port != -1)
+        if (port)
         {
             char        s[16];

@@ -1821,7 +1820,6 @@ help(void)
     printf(_("Options for \"temp-install\" mode:\n"));
     printf(_("  --no-locale               use C locale\n"));
     printf(_("  --top-builddir=DIR        (relative) path to top level build directory\n"));
-    printf(_("  --temp-port=PORT          port number to start temp postmaster on\n"));
     printf(_("  --temp-config=PATH        append contents of PATH to temporary config\n"));
     printf(_("\n"));
     printf(_("Options for using an existing installation:\n"));
@@ -1859,7 +1857,6 @@ regression_main(int argc, char *argv[],
         {"temp-install", required_argument, NULL, 9},
         {"no-locale", no_argument, NULL, 10},
         {"top-builddir", required_argument, NULL, 11},
-        {"temp-port", required_argument, NULL, 12},
         {"host", required_argument, NULL, 13},
         {"port", required_argument, NULL, 14},
         {"user", required_argument, NULL, 15},
@@ -1933,15 +1930,6 @@ regression_main(int argc, char *argv[],
             case 11:
                 top_builddir = strdup(optarg);
                 break;
-            case 12:
-                {
-                    int            p = atoi(optarg);
-
-                    /* Since Makefile isn't very bright, check port range */
-                    if (p >= 1024 && p <= 65535)
-                        temp_port = p;
-                }
-                break;
             case 13:
                 hostname = strdup(optarg);
                 break;
@@ -1982,8 +1970,14 @@ regression_main(int argc, char *argv[],
         optind++;
     }

-    if (temp_install)
-        port = temp_port;
+    if (temp_install && !port)
+        /*
+         * To reduce chances of interference with parallel
+         * installations, use a port number starting in the private
+         * range (49152-65535) calculated from version number and a
+         * time stamp.
+         */
+        port = 0xC000 | (((PG_VERSION_NUM / 100) << 4) & 0x3FF0) | ((int) time(NULL) & 0xF);

     inputdir = make_absolute_path(inputdir);
     outputdir = make_absolute_path(outputdir);
@@ -2157,7 +2151,7 @@ regression_main(int argc, char *argv[],
         postmaster_running = true;

         printf(_("running on port %d with pid %lu\n"),
-               temp_port, (unsigned long) postmaster_pid);
+               port, (unsigned long) postmaster_pid);
     }
     else
     {