From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
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: Mon, 19 Aug 2019 13:46:25 +0900
Message-ID: <20190819044625.GA18166@paquier.xyz> (raw)
In-Reply-To: <20190817224142.gq57z3cwt7c76dtk@alap3.anarazel.de>
References: <15964-c1753bdfed722e04@postgresql.org>
<24757.1566073861@sss.pgh.pa.us>
<20190817210025.v4bblh4lhsxajmlj@alap3.anarazel.de>
<6402.1566079145@sss.pgh.pa.us>
<20190817224142.gq57z3cwt7c76dtk@alap3.anarazel.de>
On Sat, Aug 17, 2019 at 03:41:42PM -0700, Andres Freund wrote:
> Heh, just discovered
> https://www.postgresql.org/message-id/20160921171819.1357.29774%40wrigleys.postgresql.org
> from the same reporter, where we went through this before :/
Ugh.
> On 2019-08-17 17:59:05 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> Most of the parallel code was move into bin/scripts/scripts_parallel.c -
>>> but there's still the above error check. Seems like we ought to add a
>>> ParallelSlotsMax() or such, and use that in the error check, rather than
>>> check FD_SETSIZE directly?
>>
>> Yeah, that would likely be cleaner than just responding to this directly.
>
> I'll go and do that.
Hm. I'd like to keep the dependency to select.h directly in
scripts_parallel.c, so the ParallelSlotsMax sounds like a good thing
to me so as FD_SETSIZE remains localized. That would give the
attached which does not take care of pgbench, and there is an extra
proposal in another part of this thread. Just looking at it now..
>> Indeed. I've confirmed the bug report on a local OpenBSD 6.4 build
>> (clang 6.0.0), and with "make -k" I can see that reindexdb.c fails
>> likewise. But this is unsurprising given that POSIX says that
>> FD_SETSIZE is declared by sys/select.h.
>
> Right.
Okay, then the current code is broken in this sense. It was
surprising to not see the buildfarm complain about that though.
--
Michael
Attachments:
[text/x-diff] parallel-slot-fdlimit.patch (2.3K, ../20190819044625.GA18166@paquier.xyz/2-parallel-slot-fdlimit.patch)
download | inline diff:diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.cindex 14a4f4a91c..f93f7dd5fe 100644--- a/src/bin/scripts/reindexdb.c+++ b/src/bin/scripts/reindexdb.c@@ -153,10 +153,10 @@ main(int argc, char *argv[])
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
}
- if (concurrentCons > FD_SETSIZE - 1)+ if (concurrentCons > ParallelSlotsMax())
{
pg_log_error("too many parallel jobs requested (maximum: %d)",
- FD_SETSIZE - 1);+ ParallelSlotsMax());
exit(1);
}
break;
diff --git a/src/bin/scripts/scripts_parallel.c b/src/bin/scripts/scripts_parallel.cindex ffdc4e49ef..2b571a470e 100644--- a/src/bin/scripts/scripts_parallel.c+++ b/src/bin/scripts/scripts_parallel.c@@ -94,6 +94,20 @@ 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+ * by POSIX.+ */+int+ParallelSlotsMax(void)+{+ /* leave some room for existing fds */+ return FD_SETSIZE - 10;+}+
/*
* ParallelSlotsGetIdle
* Return a connection slot that is ready to execute a command.
diff --git a/src/bin/scripts/scripts_parallel.h b/src/bin/scripts/scripts_parallel.hindex ab82c5e6a9..dd625577c6 100644--- a/src/bin/scripts/scripts_parallel.h+++ b/src/bin/scripts/scripts_parallel.h@@ -21,6 +21,8 @@ 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 d81bfa3a6b..c5c38692ed 100644--- a/src/bin/scripts/vacuumdb.c+++ b/src/bin/scripts/vacuumdb.c@@ -181,10 +181,10 @@ main(int argc, char *argv[])
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
}
- if (concurrentCons > FD_SETSIZE - 1)+ if (concurrentCons > ParallelSlotsMax())
{
pg_log_error("too many parallel jobs requested (maximum: %d)",
- FD_SETSIZE - 1);+ ParallelSlotsMax());
exit(1);
}
break;
[application/pgp-signature] signature.asc (832B, ../20190819044625.GA18166@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, 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: <20190819044625.GA18166@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