Peter Eisentraut wrote:
Alvaro Herrera wrote:
Is it possible to make it retry in case the chosen port is busy?  I
guess a simple check should suffice, ignoring the obvious race condition
that someone uses the port after you checked it was OK.

Well, the whole point of this exercise was to avoid that. If we had a way to do a "simple check", we might as well stick to the hardcoded port and count up from that or something.

Well, duh, the checking is actually pretty simple. We just try to connect with psql to the candidate port number before starting our own postmaster and see if anyone is already there.

Patch attached.  It solves my immediate problems nicely.
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        27 Nov 2008 10:27:36 -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.53
diff -u -3 -p -r1.53 pg_regress.c
--- src/test/regress/pg_regress.c       26 Nov 2008 13:26:52 -0000      1.53
+++ src/test/regress/pg_regress.c       27 Nov 2008 10:27:36 -0000
@@ -83,10 +83,10 @@ 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 bool port_specified_by_user = false;
 static char *dlpath = PKGLIBDIR;
 static char *user = NULL;
 static _stringlist *extraroles = NULL;
@@ -1844,7 +1844,7 @@ 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(_("  --port=PORT               start postmaster on PORT\n"));
        printf(_("  --temp-config=PATH        append contents of PATH to 
temporary config\n"));
        printf(_("\n"));
        printf(_("Options for using an existing installation:\n"));
@@ -1867,6 +1867,7 @@ regression_main(int argc, char *argv[], 
        int                     i;
        int                     option_index;
        char            buf[MAXPGPATH * 4];
+       char            buf2[MAXPGPATH * 4];
 
        static struct option long_options[] = {
                {"help", no_argument, NULL, 'h'},
@@ -1882,7 +1883,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},
@@ -1956,20 +1956,12 @@ 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;
                        case 14:
                                port = atoi(optarg);
+                               port_specified_by_user = true;
                                break;
                        case 15:
                                user = strdup(optarg);
@@ -2005,8 +1997,13 @@ regression_main(int argc, char *argv[], 
                optind++;
        }
 
-       if (temp_install)
-               port = temp_port;
+       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);
 
        inputdir = make_absolute_path(inputdir);
        outputdir = make_absolute_path(outputdir);
@@ -2107,6 +2104,37 @@ regression_main(int argc, char *argv[], 
                }
 
                /*
+                * Check if there is a postmaster running already.
+                */
+               snprintf(buf2, sizeof(buf2),
+                                SYSTEMQUOTE "\"%s/psql\" -X postgres <%s 2>%s" 
SYSTEMQUOTE,
+                                bindir, DEVNULL, DEVNULL);
+
+               for (i = 0; i < 16; i++)
+               {
+                       if (system(buf2) == 0)
+                       {
+                               char            s[16];
+
+                               if (port_specified_by_user || i == 15)
+                               {
+                                       fprintf(stderr, _("port %d apparently 
in use\n"), port);
+                                       if (!port_specified_by_user)
+                                               fprintf(stderr, _("%s: could 
not determine an available port\n"), progname);
+                                       fprintf(stderr, _("Specify an used port 
using the --port option or shut down any conflicting PostgreSQL servers.\n"));
+                                       exit_nicely(2);
+                               }
+
+                               fprintf(stderr, _("port %d apparently in use, 
trying %d\n"), port, port+1);
+                               port++;
+                               sprintf(s, "%d", port);
+                               doputenv("PGPORT", s);
+                       }
+                       else
+                               break;
+               }
+
+               /*
                 * Start the temp postmaster
                 */
                header(_("starting postmaster"));
@@ -2129,13 +2157,10 @@ regression_main(int argc, char *argv[], 
                 * second or so, but Cygwin is reportedly *much* slower).  
Don't wait
                 * forever, however.
                 */
-               snprintf(buf, sizeof(buf),
-                                SYSTEMQUOTE "\"%s/psql\" -X postgres <%s 2>%s" 
SYSTEMQUOTE,
-                                bindir, DEVNULL, DEVNULL);
                for (i = 0; i < 60; i++)
                {
                        /* Done if psql succeeds */
-                       if (system(buf) == 0)
+                       if (system(buf2) == 0)
                                break;
 
                        /*
@@ -2180,7 +2205,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
        {
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to