pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Rady, Doug <radydoug@amazon.com>
To: Andres Freund <andres@anarazel.de>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: PATCH: pgbench - option to build using ppoll() for larger connection counts
Date: Wed, 21 Mar 2018 05:42:33 +0000
Message-ID: <2879AF01-915B-4D07-9F42-04F7CA71908D@amazon.com> (raw )
In-Reply-To: <20180304001409.tajjmefj7yq6xs77@alap3.anarazel.de >
References: <23D017C9-81B7-484D-8490-FD94DEC4DF59@amazon.com >
<alpine.DEB.2.20.1801240847010.2307@lancre >
<16744C85-E44E-4354-9E3D-45A7C9BDB19B@amazon.com >
<alpine.DEB.2.20.1801252339520.26762@lancre >
<81936381-841A-459D-9FE5-18659680355C@amazon.com >
<alpine.DEB.2.20.1801262348420.17596@lancre >
<654A1247-8E1E-4E9C-937D-9119B1731FF2@amazon.com >
<20180301083404.z2sbnjdaw7gxvz6x@alap3.anarazel.de >
<alpine.DEB.2.20.1803011121300.7903@lancre >
<20180304001409.tajjmefj7yq6xs77@alap3.anarazel.de >
Updated the patch to not do the #undef
pgbench11-ppoll-v11.patch attached.
Thanks,
doug
On 3/3/18, 16:14, "Andres Freund" <andres@anarazel.de> wrote:
On 2018-03-01 11:30:39 +0100, Fabien COELHO wrote:
>
> > > -#ifdef HAVE_SYS_SELECT_H
> > > +#ifdef PGBENCH_USE_SELECT /* force use of select(2)? */
> > > +#undef HAVE_PPOLL
> > > +#endif
> > > +#ifdef HAVE_PPOLL
> > > +#include <poll.h>
> > > +#elif defined(HAVE_SYS_SELECT_H)
> > > +#define POLL_USING_SELECT
> >
> > (random thing noticed while going through patches)
> >
> > It strikes me as a bad idea to undefine configure selected
> > symbols. Postgres header might rely on them. It also strikes me as
> > entirely unnecessary here.
>
> Yes, I though about this one but let it pass. Indeed, it would be sufficient
> to not load "poll.h" when select is forced, without undefining the configure
> setting.
I've marked the CF entry waiting on author.
Greetings,
Andres Freund
Attachments:
[application/octet-stream] pgbench11-ppoll-v11.patch (10.3K, ../2879AF01-915B-4D07-9F42-04F7CA71908D@amazon.com/2-pgbench11-ppoll-v11.patch)
download | inline diff:
--- a/configure.in
+++ b/configure.in
@@ -1520,7 +1520,7 @@ PGAC_FUNC_WCSTOMBS_L
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-AC_CHECK_FUNCS([cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range utime utimes wcstombs_l])
+AC_CHECK_FUNCS([cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range utime utimes wcstombs_l])
AC_REPLACE_FUNCS(fseeko)
case $host_os in
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 29d69de4d1..45177af5ac 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -44,9 +44,18 @@
#include <signal.h>
#include <time.h>
#include <sys/time.h>
+#ifndef PGBENCH_USE_SELECT /* force use of select(2)? */
+#ifdef HAVE_PPOLL
+#define POLL_USING_PPOLL
+#include <poll.h>
+#endif
+#endif
+#ifndef POLL_USING_PPOLL
#ifdef HAVE_SYS_SELECT_H
+#define POLL_USING_SELECT
#include <sys/select.h>
#endif
+#endif
#ifdef HAVE_SYS_RESOURCE_H
#include <sys/resource.h> /* for getrlimit */
@@ -82,13 +91,19 @@ static int pthread_join(pthread_t th, void **thread_return);
/********************************************************************
* some configurable parameters */
-
-/* max number of clients allowed */
+#ifdef POLL_USING_SELECT /* using select(2) */
+#define SOCKET_WAIT_METHOD "select"
+typedef fd_set socket_set;
#ifdef FD_SETSIZE
-#define MAXCLIENTS (FD_SETSIZE - 10)
+#define MAXCLIENTS (FD_SETSIZE - 10) /* system limited max number of clients allowed */
#else
-#define MAXCLIENTS 1024
+#define MAXCLIENTS 1024 /* max number of clients allowed */
#endif
+#else /* using ppoll(2) */
+#define SOCKET_WAIT_METHOD "ppoll"
+typedef struct pollfd socket_set;
+#define MAXCLIENTS -1 /* unlimited number of clients */
+#endif /* POLL_USING_SELECT */
#define DEFAULT_INIT_STEPS "dtgvp" /* default -I setting */
@@ -503,6 +518,13 @@ static void addScript(ParsedScript script);
static void *threadRun(void *arg);
static void setalarm(int seconds);
static void finishCon(CState *st);
+static socket_set *alloc_socket_set(int count);
+static bool error_on_socket(socket_set *sa, int idx, PGconn *con);
+static void free_socket_set(socket_set *sa);
+static bool ignore_socket(socket_set *sa, int idx, PGconn *con);
+static void clear_socket_set(socket_set *sa, int count);
+static void set_socket(socket_set *sa, int fd, int idx);
+static int wait_on_socket_set(socket_set *sa, int nstate, int maxsock, int64 usec);
/* callback functions for our flex lexer */
@@ -1072,6 +1094,7 @@ doConnect(void)
!have_password)
{
PQfinish(conn);
+ conn = NULL;
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
new_pass = true;
@@ -2706,6 +2729,7 @@ doCustom(TState *thread, CState *st, StatsData *agg)
if (INSTR_TIME_IS_ZERO(now))
INSTR_TIME_SET_CURRENT(now);
start = now;
+
if ((st->con = doConnect()) == NULL)
{
fprintf(stderr, "client %d aborted while establishing connection\n",
@@ -4498,7 +4522,7 @@ main(int argc, char **argv)
case 'c':
benchmarking_option_set = true;
nclients = atoi(optarg);
- if (nclients <= 0 || nclients > MAXCLIENTS)
+ if (nclients <= 0 || (MAXCLIENTS != -1 && nclients > MAXCLIENTS))
{
fprintf(stderr, "invalid number of clients: \"%s\"\n",
optarg);
@@ -5180,6 +5204,7 @@ threadRun(void *arg)
int64 next_report = last_report + (int64) progress * 1000000;
StatsData last,
aggs;
+ socket_set *sockets = alloc_socket_set(nstate);
/*
* Initialize throttling rate target for all of the thread's clients. It
@@ -5223,6 +5248,7 @@ threadRun(void *arg)
{
if ((state[i].con = doConnect()) == NULL)
goto done;
+ set_socket(sockets, PQsocket(state[i].con), i);
}
}
@@ -5239,13 +5265,13 @@ threadRun(void *arg)
/* loop till all clients have terminated */
while (remains > 0)
{
- fd_set input_mask;
int maxsock; /* max socket number to be waited for */
int64 min_usec;
int64 now_usec = 0; /* set this only if needed */
/* identify which client sockets should be checked for input */
- FD_ZERO(&input_mask);
+ clear_socket_set(sockets, nstate);
+
maxsock = -1;
min_usec = PG_INT64_MAX;
for (i = 0; i < nstate; i++)
@@ -5294,7 +5320,7 @@ threadRun(void *arg)
goto done;
}
- FD_SET(sock, &input_mask);
+ set_socket(sockets, sock, i);
if (maxsock < sock)
maxsock = sock;
}
@@ -5331,7 +5357,7 @@ threadRun(void *arg)
/*
* If no clients are ready to execute actions, sleep until we receive
* data from the server, or a nap-time specified in the script ends,
- * or it's time to print a progress report. Update input_mask to show
+ * or it's time to print a progress report. Update sockets to show
* which client(s) received data.
*/
if (min_usec > 0)
@@ -5342,11 +5368,7 @@ threadRun(void *arg)
{
if (maxsock != -1)
{
- struct timeval timeout;
-
- timeout.tv_sec = min_usec / 1000000;
- timeout.tv_usec = min_usec % 1000000;
- nsocks = select(maxsock + 1, &input_mask, NULL, NULL, &timeout);
+ nsocks = wait_on_socket_set(sockets, nstate, maxsock, min_usec);
}
else /* nothing active, simple sleep */
{
@@ -5355,7 +5377,7 @@ threadRun(void *arg)
}
else /* no explicit delay, select without timeout */
{
- nsocks = select(maxsock + 1, &input_mask, NULL, NULL, NULL);
+ nsocks = wait_on_socket_set(sockets, nstate, maxsock, 0);
}
if (nsocks < 0)
@@ -5366,7 +5388,7 @@ threadRun(void *arg)
continue;
}
/* must be something wrong */
- fprintf(stderr, "select() failed: %s\n", strerror(errno));
+ fprintf(stderr, "%s() failed: %s\n", SOCKET_WAIT_METHOD, strerror(errno));
goto done;
}
}
@@ -5375,7 +5397,7 @@ threadRun(void *arg)
/* min_usec == 0, i.e. something needs to be executed */
/* If we didn't call select(), don't try to read any data */
- FD_ZERO(&input_mask);
+ clear_socket_set(sockets, nstate);
}
/* ok, advance the state machine of each connection */
@@ -5386,16 +5408,11 @@ threadRun(void *arg)
if (st->state == CSTATE_WAIT_RESULT)
{
/* don't call doCustom unless data is available */
- int sock = PQsocket(st->con);
- if (sock < 0)
- {
- fprintf(stderr, "invalid socket: %s",
- PQerrorMessage(st->con));
+ if (error_on_socket(sockets, i, st->con))
goto done;
- }
- if (!FD_ISSET(sock, &input_mask))
+ if (ignore_socket(sockets, i, st->con))
continue;
}
else if (st->state == CSTATE_FINISHED ||
@@ -5533,6 +5550,8 @@ done:
fclose(thread->logfile);
thread->logfile = NULL;
}
+ free_socket_set(sockets);
+ sockets = NULL;
return NULL;
}
@@ -5546,6 +5565,138 @@ finishCon(CState *st)
}
}
+#ifdef POLL_USING_SELECT /* select(2) based socket polling */
+static socket_set *
+alloc_socket_set(int count)
+{
+ return (socket_set *) pg_malloc0(sizeof(socket_set));
+}
+
+static void
+free_socket_set(socket_set *sa)
+{
+ pg_free(sa);
+}
+
+static bool
+error_on_socket(socket_set *sa, int idx, PGconn *con)
+{
+ if (PQsocket(con) >= 0) return false;
+ fprintf(stderr, "invalid socket: %s", PQerrorMessage(con));
+ return true;
+}
+
+static bool
+ignore_socket(socket_set *sa, int idx, PGconn *con)
+{
+ if (FD_ISSET(PQsocket(con), sa))
+ return false;
+ else
+ return true;
+}
+
+static void
+clear_socket_set(socket_set *sa, int count)
+{
+ FD_ZERO(sa);
+}
+
+static void
+set_socket(socket_set *sa, int fd, int idx)
+{
+ FD_SET(fd, sa);
+}
+
+static int
+wait_on_socket_set(socket_set *sa, int nstate, int maxsock, int64 usec)
+{
+ struct timeval timeout;
+
+ if (usec)
+ {
+ timeout.tv_sec = usec / 1000000;
+ timeout.tv_usec = usec % 1000000;
+ } else {
+ timeout.tv_sec = timeout.tv_usec = 0;
+ }
+
+ return select(maxsock + 1, sa, NULL, NULL, &timeout);
+}
+#else /* ppoll(2) based socket polling */
+/* ppoll() will block until timeout or one of POLL_EVENTS occurs. */
+#define POLL_EVENTS (POLLRDHUP|POLLIN|POLLPRI)
+/* ppoll() events returned that we do not want/expect to see. */
+#define POLL_UNWANTED (POLLRDHUP|POLLERR|POLLHUP|POLLNVAL)
+
+static socket_set *
+alloc_socket_set(int count)
+{
+ return (socket_set *) pg_malloc0(sizeof(socket_set) * count);
+}
+
+static void
+free_socket_set(socket_set *sa)
+{
+ pg_free(sa);
+}
+
+static bool
+error_on_socket(socket_set *sa, int idx, PGconn *con)
+{
+ /*
+ * No error if socket not used or non-error status from PQsocket() and none
+ * of the unwanted ppoll() return events.
+ */
+ if (sa[idx].fd == -1 || (PQsocket(con) >= 0 && !(sa[idx].revents & POLL_UNWANTED)))
+ return false;
+ fprintf(stderr, "invalid socket: %s", PQerrorMessage(con));
+ if (debug)
+ fprintf(stderr, "ppoll() fail - errno: %d, socket: %d, events: %x\n",
+ errno, sa[idx].fd, (sa[idx].revents & POLL_UNWANTED));
+ return true;
+}
+
+static bool
+ignore_socket(socket_set *sa, int idx, PGconn *con)
+{
+ if (sa[idx].fd != -1 && !sa[idx].revents)
+ return true;
+ else
+ return false;
+}
+
+static void
+clear_socket_set(socket_set *sa, int count)
+{
+ int i = 0;
+ for (i = 0; i < count; i++)
+ set_socket(sa, -1, i);
+}
+
+static void
+set_socket(socket_set *sa, int fd, int idx)
+{
+ sa[idx].fd = fd;
+ sa[idx].events = POLL_EVENTS;
+ sa[idx].revents = 0;
+}
+
+static int
+wait_on_socket_set(socket_set *sa, int nstate, int maxsock, int64 usec)
+{
+ struct timespec timeout;
+
+ if (usec)
+ {
+ timeout.tv_sec = usec / 1000000;
+ timeout.tv_nsec = usec % 1000000000;
+ } else {
+ timeout.tv_sec = timeout.tv_nsec = 0;
+ }
+ return ppoll(sa, nstate, &timeout, NULL);
+}
+#endif /* PGBENCH_USE_SELECT */
+
/*
* Support for duration option: set timer_exceeded after so many seconds.
*/
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c5db5430b9..a12a08892d 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -425,6 +425,9 @@
/* Define to 1 if the assembler supports PPC's LWARX mutex hint bit. */
#undef HAVE_PPC_LWARX_MUTEX_HINT
+/* Define to 1 if you have the `ppoll' function. */
+#undef HAVE_PPOLL
+
/* Define to 1 if you have the `pstat' function. */
#undef HAVE_PSTAT
view thread (51+ messages) latest in thread
Message-ID: <2879AF01-915B-4D07-9F42-04F7CA71908D@amazon.com>
Permalink: ../2879AF01-915B-4D07-9F42-04F7CA71908D@amazon.com/
Also on: postgresql.org/message-id/2879AF01-915B-4D07-9F42-04F7CA71908D@amazon.com
copy link · copy postgr.es · copy git am
reply 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-hackers@postgresql.org
Cc: radydoug@amazon.com, andres@anarazel.de, coelho@cri.ensmp.fr, robertmhaas@gmail.com
Subject: Re: PATCH: pgbench - option to build using ppoll() for larger connection counts
In-Reply-To: <2879AF01-915B-4D07-9F42-04F7CA71908D@amazon.com>
* 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