fix parallel processing of requests in gotwebd

Run just one server process per server declared in gotwebd.conf, instead of running additional sefver processes based on the "prefork" setting. The extra sefver processes weren't actually used since only one process would manage to accept a connection. The others would all fail in accept4() with EWOULDBLOCK and go back to sleep. So only the first server process will kick off work to its dedicated gotweb sibling, immediately win the accept race(?) again, and then wait for its gotweb worker to process current and the next request. The other processes all remain idle, at least as far as I have observed behaviour on a single-CPU system. 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. This can be tested by running a difficult blame request (e.g. blame got/got.c in got.git) and browsing other gotwebd pages in other tabs. The "prefork" setting now only controls the number of gotweb workers which will be started. In the future, I plan to improve the round-robin strategy since it can still lead to requests waiting for a busy worker while other workers are idle. This requires tracking the state of requests in sockets.c to know when work has completed. But the current gotwebd code no longer does such state tracking. My other pending change to split the parsing of FCGI parameters into a separate process reintroduces the state tracking for unrelated reasons. We could then use this to improve load-balancing, too. Ok? M gotwebd/config.c | 2+ 2- M gotwebd/fcgi.c | 8+ 2- M gotwebd/gotweb.c | 26+ 15- M gotwebd/gotwebd.c | 48+ 23- M gotwebd/gotwebd.conf.5 | 6+ 2- M gotwebd/gotwebd.h | 2+ 2- M gotwebd/parse.y | 1+ 1- M gotwebd/sockets.c | 21+ 11- 8 files changed, 114 insertions(+), 58 deletions(-) commit - 6a5431286b29f74a150aed3c2cb14c6744401e11 commit + f5cd2dea7138c00c955e64e52057be1435fb25d2 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 + ff291655260fed9fdd39ec952d797480fd737156 --- 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,16 @@ 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; + log_info("dispatch to gotweb %d via fd %d", env->gotweb_cur, iev_gotweb->ibuf.fd); + + 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 + 1489f7bf4879e67af7d43f2ab53d6bdb7885a34d --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -1375,13 +1375,16 @@ 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,19 +1532,17 @@ 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; } + fd = imsg_get_fd(imsg); 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 +1552,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 +1630,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 +1642,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);

2025-09-05 15:20 Stefan Sperling:
fix parallel processing of requests in gotwebd
- 2025-09-06 10:06 Omar Polo:
  fix parallel processing of requests in gotwebd