commit 0aa852e7850dc0a27b20c9aed9e07ab39d024308 from: Stefan Sperling via: Thomas Adam date: Sat Sep 06 16:29:36 2025 UTC 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@ 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);