Commit Diff

Commit:
0aa852e7850dc0a27b20c9aed9e07ab39d024308
From:
Stefan Sperling <stsp@stsp.name>
Via:
Thomas Adam <thomas.adam22@gmail.com>
Date:
Message:
fix parallel processing of requests in gotwebd Run just one server process per server declared in gotwebd.conf, instead of running additional server processes based on the "prefork" setting. The extra servers weren't actually used since they would all wake up together, only one of them would manage to accept a connection, and the others would fail wtth EWOULDBLOCK go back to sleep instead of handling other connections. Having a single listening process dispatch request across gotweb processes in a round-robin fashion actually allows requests to be processed in parallel as intended. We currently use a round-robin scheme which can still cause some requests to wait for a busy worker. This will be improved later. The "prefork" setting now only controls the number of gotweb workers which will be started. ok op@
Actions:
Patch | Tree
commit - 89898c2ae7a615f3a6d4de6ca048416f58be0dae
commit + 0aa852e7850dc0a27b20c9aed9e07ab39d024308
blob - d288a65f184ba19f1c32db03d1fc737834118187
blob + a2d3c178a39430c8339c1c8563b6f7514ea42f64
--- gotwebd/config.c
+++ gotwebd/config.c
@@ -49,7 +49,7 @@ config_init(struct gotwebd *env)
 
 	strlcpy(env->httpd_chroot, D_HTTPD_CHROOT, sizeof(env->httpd_chroot));
 
-	env->prefork_gotwebd = GOTWEBD_NUMPROC;
+	env->prefork = GOTWEBD_NUMPROC;
 	env->server_cnt = 0;
 	TAILQ_INIT(&env->servers);
 	TAILQ_INIT(&env->sockets);
@@ -162,7 +162,7 @@ config_setfd(struct gotwebd *env)
 	    __func__, PRIV_FDS__MAX + GOTWEB_PACK_NUM_TEMPFILES);
 
 	for (i = 0; i < PRIV_FDS__MAX + GOTWEB_PACK_NUM_TEMPFILES; i++) {
-		for (j = 0; j < env->nserver; ++j) {
+		for (j = 0; j < env->prefork; j++) {
 			fd = got_opentempfd();
 			if (fd == -1)
 				fatal("got_opentemp");
blob - 16c1a4c345629c57a011c73d856df48152069120
blob + df7dbf5edcdbb2ef043b8f615d0f94fd6db61d81
--- gotwebd/fcgi.c
+++ gotwebd/fcgi.c
@@ -200,6 +200,7 @@ static void
 process_request(struct request *c)
 {
 	struct gotwebd *env = gotwebd_env;
+	struct imsgev *iev_gotweb;
 	int ret, i;
 	struct request ic;
 
@@ -218,11 +219,15 @@ process_request(struct request *c)
 		ic.priv_fd[i] = -1;
 	ic.fd = -1;
 
-	ret = imsg_compose_event(env->iev_gotweb, GOTWEBD_IMSG_REQ_PROCESS,
+	/* Round-robin requests across gotweb processes. */
+	iev_gotweb = &env->iev_gotweb[env->gotweb_cur];
+	env->gotweb_cur = (env->gotweb_cur + 1) % env->prefork;
+
+	ret = imsg_compose_event(iev_gotweb, GOTWEBD_IMSG_REQ_PROCESS,
 	    GOTWEBD_PROC_SERVER, -1, c->fd, &ic, sizeof(ic));
 	if (ret == -1) {
 		log_warn("imsg_compose_event");
-		close(c->fd);
+		return;
 	}
 	c->fd = -1;
 
blob - c091fcdb65e803da4535ffaf82debc9f8096b2c9
blob + caea940d5cb39d5d4166b7c91f3166298889432a
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -1373,12 +1373,15 @@ gotweb_render_age(struct template *tp, time_t committe
 static void
 gotweb_shutdown(void)
 {
+	struct gotwebd *env = gotwebd_env;
+	int i;
+
 	imsgbuf_clear(&gotwebd_env->iev_parent->ibuf);
 	free(gotwebd_env->iev_parent);
-	if (gotwebd_env->iev_server) {
-		imsgbuf_clear(&gotwebd_env->iev_server->ibuf);
-		free(gotwebd_env->iev_server);
-	}
+
+	for (i = 0; i < env->server_cnt - env->servers_pending; i++)
+		imsgbuf_clear(&gotwebd_env->iev_server[i].ibuf);
+	free(gotwebd_env->iev_server);
 
 	while (!TAILQ_EMPTY(&gotwebd_env->servers)) {
 		struct server *srv;
@@ -1430,8 +1433,9 @@ gotweb_launch(struct gotwebd *env)
 {
 	struct server *srv;
 	const struct got_error *error;
+	int i;
 
-	if (env->iev_server == NULL)
+	if (env->servers_pending != 0)
 		fatal("server process not connected");
 
 #ifndef PROFILE
@@ -1451,7 +1455,8 @@ gotweb_launch(struct gotwebd *env)
 	if (unveil(NULL, NULL) == -1)
 		fatal("unveil");
 
-	event_add(&env->iev_server->ev, NULL);
+	for (i = 0; i < env->server_cnt; i++)
+		event_add(&env->iev_server[i].ev, NULL);
 }
 
 static void
@@ -1525,8 +1530,8 @@ recv_server_pipe(struct gotwebd *env, struct imsg *ims
 	struct imsgev *iev;
 	int fd;
 
-	if (env->iev_server != NULL) {
-		log_warn("server pipe already received");
+	if (env->servers_pending <= 0) {
+		log_warn("server pipes already received");
 		return;
 	}
 
@@ -1534,10 +1539,7 @@ recv_server_pipe(struct gotwebd *env, struct imsg *ims
 	if (fd == -1)
 		fatalx("invalid server pipe fd");
 
-	iev = calloc(1, sizeof(*iev));
-	if (iev == NULL)
-		fatal("calloc");
-
+	iev = &env->iev_server[env->servers_pending - 1];
 	if (imsgbuf_init(&iev->ibuf, fd) == -1)
 		fatal("imsgbuf_init");
 	imsgbuf_allow_fdpass(&iev->ibuf);
@@ -1547,7 +1549,7 @@ recv_server_pipe(struct gotwebd *env, struct imsg *ims
 	event_set(&iev->ev, fd, EV_READ, gotweb_dispatch_server, iev);
 	imsg_event_add(iev);
 
-	env->iev_server = iev;
+	env->servers_pending--;
 }
 
 static void
@@ -1625,8 +1627,9 @@ gotweb(struct gotwebd *env, int fd)
 
 	sockets_rlimit(-1);
 
-	if ((env->iev_parent = malloc(sizeof(*env->iev_parent))) == NULL)
-		fatal("malloc");
+	env->iev_parent = calloc(1, sizeof(*env->iev_parent));
+	if (env->iev_parent == NULL)
+		fatal("calloc");
 	if (imsgbuf_init(&env->iev_parent->ibuf, fd) == -1)
 		fatal("imsgbuf_init");
 	imsgbuf_allow_fdpass(&env->iev_parent->ibuf);
@@ -1636,6 +1639,13 @@ gotweb(struct gotwebd *env, int fd)
 	    env->iev_parent);
 	event_add(&env->iev_parent->ev, NULL);
 
+	if (env->server_cnt <= 0)
+		fatalx("invalid server count: %d", env->server_cnt);
+	env->iev_server = calloc(env->server_cnt, sizeof(*env->iev_server));
+	if (env->iev_server == NULL)
+		fatal("calloc");
+	env->servers_pending = env->server_cnt;
+
 	signal(SIGPIPE, SIG_IGN);
 
 	signal_set(&sighup, SIGHUP, gotweb_sighdlr, env);
blob - 4b4ad08212834af192f08c7a9c4bf9fe9ace7db2
blob + 27eaa7a28aeac4685d4ff2413272c0d32c1d5493
--- gotwebd/gotwebd.c
+++ gotwebd/gotwebd.c
@@ -122,7 +122,7 @@ main_compose_sockets(struct gotwebd *env, uint32_t typ
 	size_t i;
 	int ret = 0;
 
-	for (i = 0; i < env->nserver; ++i) {
+	for (i = 0; i < env->server_cnt; ++i) {
 		ret = send_imsg(&env->iev_server[i], type, fd, data, len);
 		if (ret)
 			break;
@@ -138,7 +138,7 @@ main_compose_gotweb(struct gotwebd *env, uint32_t type
 	size_t i;
 	int ret = 0;
 
-	for (i = 0; i < env->nserver; ++i) {
+	for (i = 0; i < env->prefork; i++) {
 		ret = send_imsg(&env->iev_gotweb[i], type, fd, data, len);
 		if (ret)
 			break;
@@ -283,7 +283,7 @@ spawn_process(struct gotwebd *env, const char *argv0, 
     enum gotwebd_proc_type proc_type, const char *username,
     void (*handler)(int, short, void *))
 {
-	const char	*argv[9];
+	const char	*argv[10];
 	int		 argc = 0;
 	int		 p[2];
 	pid_t		 pid;
@@ -312,11 +312,21 @@ spawn_process(struct gotwebd *env, const char *argv0, 
 
 	argv[argc++] = argv0;
 	if (proc_type == GOTWEBD_PROC_SERVER) {
+		char *s;
+
 		argv[argc++] = "-S";
 		argv[argc++] = username;
+		if (asprintf(&s, "-S%d", env->prefork) == -1)
+			fatal("asprintf");
+		argv[argc++] = s;
 	} else if (proc_type == GOTWEBD_PROC_GOTWEB) {
+		char *s;
+
 		argv[argc++] = "-G";
 		argv[argc++] = username;
+		if (asprintf(&s, "-G%d", env->server_cnt) == -1)
+			fatal("asprintf");
+		argv[argc++] = s;
 	}
 	if (strcmp(env->gotwebd_conffile, GOTWEBD_CONF) != 0) {
 		argv[argc++] = "-f";
@@ -364,7 +374,7 @@ main(int argc, char **argv)
 	const char		*www_username = GOTWEBD_WWW_USER;
 	gid_t			 gotwebd_groups[NGROUPS_MAX];
 	gid_t			 www_gid;
-	const char		*argv0;
+	const char		*argv0, *errstr;
 
 	if ((argv0 = argv[0]) == NULL)
 		argv0 = "gotwebd";
@@ -389,7 +399,11 @@ main(int argc, char **argv)
 			break;
 		case 'G':
 			proc_type = GOTWEBD_PROC_GOTWEB;
-			gotwebd_username = optarg;
+			i = strtonum(optarg, 1, INT_MAX, &errstr);
+			if (errstr)
+				gotwebd_username = optarg;
+			else
+				env->server_cnt = i;
 			break;
 		case 'f':
 			conffile = optarg;
@@ -399,7 +413,11 @@ main(int argc, char **argv)
 			break;
 		case 'S':
 			proc_type = GOTWEBD_PROC_SERVER;
-			gotwebd_username = optarg;
+			i = strtonum(optarg, 1, INT_MAX, &errstr);
+			if (errstr)
+				gotwebd_username = optarg;
+			else
+				env->prefork = i;
 			break;
 		case 'v':
 			if (env->gotwebd_verbose < 3)
@@ -488,18 +506,19 @@ main(int argc, char **argv)
 
 	evb = event_init();
 
-	env->nserver = env->prefork_gotwebd;
-	env->iev_server = calloc(env->nserver, sizeof(*env->iev_server));
+	env->iev_server = calloc(env->server_cnt, sizeof(*env->iev_server));
 	if (env->iev_server == NULL)
 		fatal("calloc");
-	env->iev_gotweb = calloc(env->nserver, sizeof(*env->iev_gotweb));
+	env->iev_gotweb = calloc(env->prefork, sizeof(*env->iev_gotweb));
 	if (env->iev_gotweb == NULL)
 		fatal("calloc");
 
-	for (i = 0; i < env->nserver; ++i) {
+	for (i = 0; i < env->server_cnt; ++i) {
 		spawn_process(env, argv0, &env->iev_server[i],
 		    GOTWEBD_PROC_SERVER, gotwebd_username,
 		    gotwebd_dispatch_server);
+	}
+	for (i = 0; i < env->prefork; ++i) {
 		spawn_process(env, argv0, &env->iev_gotweb[i],
 		    GOTWEBD_PROC_GOTWEB, gotwebd_username,
 		    gotwebd_dispatch_gotweb);
@@ -560,20 +579,26 @@ connect_children(struct gotwebd *env)
 {
 	struct imsgev *iev1, *iev2;
 	int pipe[2];
-	int i;
+	int i, j;
 
-	for (i = 0; i < env->nserver; i++) {
+	for (i = 0; i < env->server_cnt; i++) {
 		iev1 = &env->iev_server[i];
-		iev2 = &env->iev_gotweb[i];
 
-		if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe) == -1)
-			fatal("socketpair");
+		for (j = 0; j < env->prefork; j++) {
+			iev2 = &env->iev_gotweb[j];
 
-		if (send_imsg(iev1, GOTWEBD_IMSG_CTL_PIPE, pipe[0], NULL, 0))
-			fatal("send_imsg");
+			if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC,
+			    pipe) == -1)
+				fatal("socketpair");
 
-		if (send_imsg(iev2, GOTWEBD_IMSG_CTL_PIPE, pipe[1], NULL, 0))
-			fatal("send_imsg");
+			if (send_imsg(iev1, GOTWEBD_IMSG_CTL_PIPE, pipe[0],
+			    NULL, 0))
+				fatal("send_imsg");
+
+			if (send_imsg(iev2, GOTWEBD_IMSG_CTL_PIPE, pipe[1],
+			    NULL, 0))
+				fatal("send_imsg");
+		}
 	}
 }
 
@@ -584,8 +609,8 @@ gotwebd_configure(struct gotwebd *env, uid_t uid, gid_
 	struct socket *sock;
 
 	/* gotweb need to reload its config. */
-	env->servers_pending = env->prefork_gotwebd;
-	env->gotweb_pending = env->prefork_gotwebd;
+	env->servers_pending = env->server_cnt;
+	env->gotweb_pending = env->prefork;
 
 	/* send our gotweb servers */
 	TAILQ_FOREACH(srv, &env->servers, entry) {
@@ -643,7 +668,7 @@ gotwebd_shutdown(void)
 	pid_t		 pid;
 	int		 i, status;
 
-	for (i = 0; i < env->nserver; ++i) {
+	for (i = 0; i < env->server_cnt; ++i) {
 		event_del(&env->iev_server[i].ev);
 		imsgbuf_clear(&env->iev_server[i].ibuf);
 		close(env->iev_server[i].ibuf.fd);
@@ -651,7 +676,7 @@ gotwebd_shutdown(void)
 	}
 	free(env->iev_server);
 
-	for (i = 0; i < env->nserver; ++i) {
+	for (i = 0; i < env->prefork; ++i) {
 		event_del(&env->iev_gotweb[i].ev);
 		imsgbuf_clear(&env->iev_gotweb[i].ibuf);
 		close(env->iev_gotweb[i].ibuf.fd);
blob - 562d13bf30399506f8f4f2c9eca0d103cf6e5cdd
blob + 27e26b6abdbf45670ce6655d54bb9c73a73074ea
--- gotwebd/gotwebd.conf.5
+++ gotwebd/gotwebd.conf.5
@@ -81,9 +81,13 @@ While the specified
 must be absolute, it should usually point inside the web server's chroot
 directory such that the web server can access the socket.
 .It Ic prefork Ar number
-Run the specified number of server processes.
+Spawn enough processes such that
+.Ar number
+requests can be handled in parallel.
+By default,
 .Xr gotwebd 8
-runs 3 server processes by default.
+will handle up to 3 requests in parallel.
+The maximum allowed is 32.
 .It Ic user Ar user
 Set the
 .Ar user
blob - 888f71c5b60cbc3697f03efa568436627738f7dc
blob + b6e69183db3f973c8495e874e281a672de6f3f4c
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -368,11 +368,11 @@ struct gotwebd {
 	struct imsgev	*iev_parent;
 	struct imsgev	*iev_server;
 	struct imsgev	*iev_gotweb;
-	size_t		 nserver;
 
-	uint16_t	 prefork_gotwebd;
+	uint16_t	 prefork;
 	int		 servers_pending;
 	int		 gotweb_pending;
+	int		 gotweb_cur;
 
 	int		 server_cnt;
 
blob - 35fc0ecdfbd4c31e8ff97485dc137cb8f39f90d2
blob + 2b6d5d8a0700920880d085ff2a285e59721e6da1
--- gotwebd/parse.y
+++ gotwebd/parse.y
@@ -185,7 +185,7 @@ main		: PREFORK NUMBER {
 				    $2 <= 0 ? "too small" : "too large", $2);
 				YYERROR;
 			}
-			gotwebd->prefork_gotwebd = $2;
+			gotwebd->prefork = $2;
 		}
 		| CHROOT STRING {
 			if (*$2 == '\0') {
blob - 3bb373f4344d0faa63a41e1a402450610f836e23
blob + 42b3858588b58a7e52039e2b9a049d1e4ea22cd8
--- gotwebd/sockets.c
+++ gotwebd/sockets.c
@@ -89,8 +89,9 @@ sockets(struct gotwebd *env, int fd)
 
 	sockets_rlimit(-1);
 
-	if ((env->iev_parent = malloc(sizeof(*env->iev_parent))) == NULL)
-		fatal("malloc");
+	env->iev_parent = calloc(1, sizeof(*env->iev_parent));
+	if (env->iev_parent == NULL)
+		fatal("calloc");
 	if (imsgbuf_init(&env->iev_parent->ibuf, fd) == -1)
 		fatal("imsgbuf_init");
 	imsgbuf_allow_fdpass(&env->iev_parent->ibuf);
@@ -100,6 +101,13 @@ sockets(struct gotwebd *env, int fd)
 	    env->iev_parent);
 	event_add(&env->iev_parent->ev, NULL);
 
+	if (env->prefork <= 0)
+		fatalx("invalid prefork count: %d", env->prefork);
+	env->iev_gotweb = calloc(env->prefork, sizeof(*env->iev_gotweb));
+	if (env->iev_gotweb == NULL)
+		fatal("calloc");
+	env->gotweb_pending = env->prefork;
+
 	signal(SIGPIPE, SIG_IGN);
 
 	signal_set(&sighup, SIGHUP, sockets_sighdlr, env);
@@ -188,8 +196,9 @@ static void
 sockets_launch(struct gotwebd *env)
 {
 	struct socket *sock;
+	int i;
 
-	if (env->iev_gotweb == NULL)
+	if (env->gotweb_pending != 0)
 		fatal("gotweb process not connected");
 
 	TAILQ_FOREACH(sock, &gotwebd_env->sockets, entry) {
@@ -212,7 +221,8 @@ sockets_launch(struct gotwebd *env)
 	if (pledge("stdio inet sendfd", NULL) == -1)
 		fatal("pledge");
 #endif
-	event_add(&env->iev_gotweb->ev, NULL);
+	for (i = 0; i < env->prefork; i++)
+		event_add(&env->iev_gotweb[i].ev, NULL);
 
 }
 
@@ -268,7 +278,7 @@ recv_gotweb_pipe(struct gotwebd *env, struct imsg *ims
 	struct imsgev *iev;
 	int fd;
 
-	if (env->iev_gotweb != NULL) {
+	if (env->gotweb_pending <= 0) {
 		log_warn("gotweb pipe already received");
 		return;
 	}
@@ -277,10 +287,7 @@ recv_gotweb_pipe(struct gotwebd *env, struct imsg *ims
 	if (fd == -1)
 		fatalx("invalid gotweb pipe fd");
 
-	iev = calloc(1, sizeof(*iev));
-	if (iev == NULL)
-		fatal("calloc");
-
+	iev = &env->iev_gotweb[env->gotweb_pending - 1];
 	if (imsgbuf_init(&iev->ibuf, fd) == -1)
 		fatal("imsgbuf_init");
 	imsgbuf_allow_fdpass(&iev->ibuf);
@@ -290,7 +297,7 @@ recv_gotweb_pipe(struct gotwebd *env, struct imsg *ims
 	event_set(&iev->ev, fd, EV_READ, server_dispatch_gotweb, iev);
 	imsg_event_add(iev);
 
-	env->iev_gotweb = iev;
+	env->gotweb_pending--;
 }
 
 static void
@@ -383,6 +390,8 @@ sockets_sighdlr(int sig, short event, void *arg)
 static void
 sockets_shutdown(void)
 {
+	int i;
+
 	/* clean servers */
 	while (!TAILQ_EMPTY(&gotwebd_env->servers)) {
 		struct server *srv;
@@ -410,7 +419,8 @@ sockets_shutdown(void)
 
 	imsgbuf_clear(&gotwebd_env->iev_parent->ibuf);
 	free(gotwebd_env->iev_parent);
-	imsgbuf_clear(&gotwebd_env->iev_gotweb->ibuf);
+	for (i = 0; i < gotwebd_env->prefork; i++)
+		imsgbuf_clear(&gotwebd_env->iev_gotweb[i].ibuf);
 	free(gotwebd_env->iev_gotweb);
 	free(gotwebd_env);