summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNoah Misch <noah@leadboat.com>2023-10-14 15:54:46 -0700
committerNoah Misch <noah@leadboat.com>2023-10-14 15:54:50 -0700
commit3895e9153e29af66b038cf17756b4307fa967718 (patch)
treeda99b509be5224f0bf857d00a74d7c9ac1863417
parent890a73ba3ac0ea9ec2ad6a828ffe03ca12b11564 (diff)
Don't spuriously report FD_SETSIZE exhaustion on Windows.
Starting on 2023-08-03, this intermittently terminated a "pgbench -C" test in CI. It could affect a high-client-count "pgbench" without "-C". While parallel reindexdb and vacuumdb reach the same problematic check, sufficient client count and/or connection turnover is less plausible for them. Given the lack of examples from the buildfarm or from manual builds, reproducing this must entail rare operating system configurations. Also correct the associated error message, which was wrong for non-Windows. Back-patch to v12, where the pgbench check first appeared. While v11 vacuumdb has the problematic check, reaching it with typical vacuumdb usage is implausible. Reviewed by Thomas Munro. Discussion: https://postgr.es/m/CA+hUKG+JwvTNdcyJTriy9BbtzF1veSRQ=9M_ZKFn9_LqE7Kp7Q@mail.gmail.com
-rw-r--r--src/bin/pgbench/pgbench.c17
-rw-r--r--src/bin/scripts/scripts_parallel.c34
2 files changed, 41 insertions, 10 deletions
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index bf5b8b7d3ef..6ea89cabc97 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6710,15 +6710,22 @@ clear_socket_set(socket_set *sa)
static void
add_socket_to_set(socket_set *sa, int fd, int idx)
{
+ /* See connect_slot() for background on this code. */
+#ifdef WIN32
+ if (sa->fds.fd_count + 1 >= FD_SETSIZE)
+ {
+ pg_log_fatal("too many concurrent database clients for this platform: %d",
+ sa->fds.fd_count + 1);
+ exit(1);
+ }
+#else
if (fd < 0 || fd >= FD_SETSIZE)
{
- /*
- * Doing a hard exit here is a bit grotty, but it doesn't seem worth
- * complicating the API to make it less grotty.
- */
- pg_log_fatal("too many client connections for select()");
+ pg_log_fatal("socket file descriptor out of range for select(): %d",
+ fd);
exit(1);
}
+#endif
FD_SET(fd, &sa->fds);
if (fd > sa->maxfd)
sa->maxfd = fd;
diff --git a/src/bin/scripts/scripts_parallel.c b/src/bin/scripts/scripts_parallel.c
index ec264a269a7..426897a87ab 100644
--- a/src/bin/scripts/scripts_parallel.c
+++ b/src/bin/scripts/scripts_parallel.c
@@ -223,15 +223,39 @@ ParallelSlotsSetup(const ConnParams *cparams,
conn = connectDatabase(cparams, progname, echo, false, true);
/*
- * Fail and exit immediately if trying to use a socket in an
- * unsupported range. POSIX requires open(2) to use the lowest
- * unused file descriptor and the hint given relies on that.
+ * POSIX defines FD_SETSIZE as the highest file descriptor
+ * acceptable to FD_SET() and allied macros. Windows defines it
+ * as a ceiling on the count of file descriptors in the set, not a
+ * ceiling on the value of each file descriptor; see
+ * https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select
+ * and
+ * https://learn.microsoft.com/en-us/windows/win32/api/winsock/ns-winsock-fd_set.
+ * We can't ignore that, because Windows starts file descriptors
+ * at a higher value, delays reuse, and skips values. With less
+ * than ten concurrent file descriptors, opened and closed
+ * rapidly, one can reach file descriptor 1024.
+ *
+ * Doing a hard exit here is a bit grotty, but it doesn't seem
+ * worth complicating the API to make it less grotty.
*/
- if (PQsocket(conn) >= FD_SETSIZE)
+#ifdef WIN32
+ if (i >= FD_SETSIZE)
{
- pg_log_fatal("too many jobs for this platform -- try %d", i);
+ pg_log_fatal("too many jobs for this platform: %d", i);
exit(1);
}
+#else
+ {
+ int fd = PQsocket(conn);
+
+ if (fd >= FD_SETSIZE)
+ {
+ pg_log_fatal("socket file descriptor out of range for select(): %d",
+ fd);
+ exit(1);
+ }
+ }
+#endif
init_slot(slots + i, conn);
}