Commit Diff

Commit:
bb32659f4666bd508118a9e15238bb69471b3b14
From:
Stefan Sperling <stsp@stsp.name>
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 - 6a5431286b29f74a150aed3c2cb14c6744401e11
commit + bb32659f4666bd508118a9e15238bb69471b3b14
blob - 90eb143afce8e50c7ba53d141294ca8616d7a074
blob + f5f240b7d414b6737f10eb222651623aef0067e3
--- 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 - 0f92cc1b3d0cdafc2c5d0232698fed51772579fe
blob + 58a88e7f43f9c77ff11bef8420d7ec926c4ffb56
--- gotwebd/fcgi.c
+++ gotwebd/fcgi.c
@@ -198,6 +198,7 @@ static void
 process_request(struct request *c)
 {
 	struct gotwebd *env = gotwebd_env;
+	struct imsgev *iev_gotweb;
 	int ret, i;
 	struct request ic;
 
@@ -216,11 +217,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 - 03f8e6941798467f7ecdd2e1de64cdc88bf28422
blob + cc891f8eb78a1444fed0684e79e52a496a2008c7
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -1375,12 +1375,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;
@@ -1432,8 +1435,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
@@ -1453,7 +1457,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
@@ -1527,8 +1532,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;
 	}
 
@@ -1536,10 +1541,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);
@@ -1549,7 +1551,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
@@ -1627,8 +1629,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);
@@ -1638,6 +1641,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 - 7e6aaf321f8905c1854851da0b0ffd2335b49ea9
blob + 68ffcc1638c3db490e1b14e4ab1194f8dc7776db
--- 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;
@@ -284,7 +284,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;
@@ -313,11 +313,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";
@@ -365,7 +375,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";
@@ -390,7 +400,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;
@@ -400,7 +414,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)
@@ -489,18 +507,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);
@@ -561,20 +580,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");
+		}
 	}
 }
 
@@ -585,8 +610,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) {
@@ -644,7 +669,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);
@@ -652,7 +677,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 - 28cc307a5e9e04f189781f0f3c7c189a87700a17
blob + a985c4c448a29a5e34d9a497c1736397cd5ed2af
--- gotwebd/parse.y
+++ gotwebd/parse.y
@@ -184,7 +184,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 - 2d3df9cf55d52a941f6e5bb9603401f44514b73d
blob + b8454b51d4dcc561ebb7dcdda35495534adc5707
--- 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);