From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: jungleboogie0@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15964: vacuumdb.c:187:10: error: use of undeclared identifier 'FD_SETSIZE'
Date: Wed, 21 Aug 2019 10:58:54 +0900
Message-ID: <20190821015854.GD1684@paquier.xyz> (raw)
In-Reply-To: <20190820160553.37nd5jxs4dk2qujy@alap3.anarazel.de>
References: <20190818001858.ho3ev4z57fqhs7a5@alap3.anarazel.de>
<20190819025953.GA15110@alvherre.pgsql>
<20190819042832.GA1702@paquier.xyz>
<23514.1566189171@sss.pgh.pa.us>
<20190819051251.GB18166@paquier.xyz>
<20190820160553.37nd5jxs4dk2qujy@alap3.anarazel.de>
On Tue, Aug 20, 2019 at 09:05:53AM -0700, Andres Freund wrote:
> On 2019-08-19 14:12:51 +0900, Michael Paquier wrote:
>> - 0002 makes vacuumdb and reindexdb fail when trying to assign a
>> socket with an unsupported range. Should this bit be backpatched? We
>> are doing that for vacuumdb for some time now, and the error is
>> confusing so I would prefer fixing it on older branches as well.
>
> Yea, I think we clearly need to. The code right now essentially is a
> wild write, albeit with a somewhat limited range of what it can impact.
Okay.
> I think the error message ought be reformulated, so users have a chance
> to actually understand what they need to change to avoid the error. At
> least something roughly like "too many jobs for this platform's select()".
pgbench needs an extra fix then?
> ISTM that we should fail in ParallelSlotsSetup(), rather than
> ParallelSlotsGetIdle() though? That's always going to be earlier, and
> there's no way to get into the problematic situation at a later point,
> no?
Okay, done this way. What do you think about the attached?
--
Michael
Attachments:
[text/x-diff] parallel-slot-fdlimit-v3.patch (3.3K, ../20190821015854.GD1684@paquier.xyz/2-parallel-slot-fdlimit-v3.patch)
download | inline diff:diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.cindex 570cf3306a..fbf1370d61 100644--- a/src/bin/pgbench/pgbench.c+++ b/src/bin/pgbench/pgbench.c@@ -6420,7 +6420,7 @@ add_socket_to_set(socket_set *sa, int fd, int idx)
* Doing a hard exit here is a bit grotty, but it doesn't seem worth
* complicating the API to make it less grotty.
*/
- fprintf(stderr, "too many client connections for select()\n");+ fprintf(stderr, "too many jobs for this platform's select()\n");
exit(1);
}
FD_SET(fd, &sa->fds);
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.cindex f93f7dd5fe..f00aec15de 100644--- a/src/bin/scripts/reindexdb.c+++ b/src/bin/scripts/reindexdb.c@@ -153,12 +153,6 @@ main(int argc, char *argv[])
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
}
- if (concurrentCons > ParallelSlotsMax())- {- pg_log_error("too many parallel jobs requested (maximum: %d)",- ParallelSlotsMax());- exit(1);- }
break;
case 'v':
verbose = true;
diff --git a/src/bin/scripts/scripts_parallel.c b/src/bin/scripts/scripts_parallel.cindex 10379a1f99..e0643bc914 100644--- a/src/bin/scripts/scripts_parallel.c+++ b/src/bin/scripts/scripts_parallel.c@@ -94,20 +94,6 @@ select_loop(int maxFd, fd_set *workerset, bool *aborting)
return i;
}
-/*- * ParallelSlotsMax- * Returns the maximum number of parallel slots supported.- *- * Note that this is included here as FD_SETSIZE is declared in sys/select.h- * per POSIX.- */-int-ParallelSlotsMax(void)-{- /* leave some room for pre-existing fds */- return FD_SETSIZE - 10;-}-
/*
* ParallelSlotsGetIdle
* Return a connection slot that is ready to execute a command.
@@ -246,6 +232,18 @@ ParallelSlotsSetup(const char *dbname, const char *host, const char *port,
{
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, echo, false, true);
++ /*+ * Fail immediately if trying to use an index in an unsupported+ * range. Doing a hard exit here is not beautiful, but that's+ * not worth complicating the logic.+ */+ if (PQsocket(conn) >= FD_SETSIZE)+ {+ fprintf(stderr, "too many jobs for this platform's select()\n");+ exit(1);+ }+
init_slot(slots + i, conn);
}
}
diff --git a/src/bin/scripts/scripts_parallel.h b/src/bin/scripts/scripts_parallel.hindex 8042345072..ab82c5e6a9 100644--- a/src/bin/scripts/scripts_parallel.h+++ b/src/bin/scripts/scripts_parallel.h@@ -21,8 +21,6 @@ typedef struct ParallelSlot
bool isFree; /* Is it known to be idle? */
} ParallelSlot;
-extern int ParallelSlotsMax(void);-
extern ParallelSlot *ParallelSlotsGetIdle(ParallelSlot *slots, int numslots);
extern ParallelSlot *ParallelSlotsSetup(const char *dbname, const char *host,
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.cindex c5c38692ed..2c7219239f 100644--- a/src/bin/scripts/vacuumdb.c+++ b/src/bin/scripts/vacuumdb.c@@ -181,12 +181,6 @@ main(int argc, char *argv[])
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
}
- if (concurrentCons > ParallelSlotsMax())- {- pg_log_error("too many parallel jobs requested (maximum: %d)",- ParallelSlotsMax());- exit(1);- }
break;
case 2:
maintenance_db = pg_strdup(optarg);
[application/pgp-signature] signature.asc (832B, ../20190821015854.GD1684@paquier.xyz/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, andres@anarazel.de, tgl@sss.pgh.pa.us, alvherre@2ndquadrant.com, dilipbalaut@gmail.com, jungleboogie0@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15964: vacuumdb.c:187:10: error: use of undeclared identifier 'FD_SETSIZE'
In-Reply-To: <20190821015854.GD1684@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox